Re: [libvirt] [PATCH] AUTHORS: Add Katerina Koukiou

2018-07-16 Thread Erik Skultety
On Mon, Jul 16, 2018 at 05:00:24PM +0200, Ján Tomko wrote:
> $ git log --format=oneline --committer=kkoukiou | wc -l
> 3
>
> Also set up the mailmap entry to avoid duplicates in the
> generated authors file.
>
> Signed-off-by: Ján Tomko 
> ---

I believe noone would have objected if you pushed this right away, but it's
fine.

Reviewed-by: Erik Skultety 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] util: set OOM in virCopyLastError if error is not set

2018-07-16 Thread Nikolay Shirokovskiy



On 17.07.2018 01:38, John Ferlan wrote:
> 
> 
> On 07/02/2018 07:33 AM, Nikolay Shirokovskiy wrote:
>> virCopyLastError is intended to be used after last error is set.
>> However due to virLastErrorObject failures (very unlikely through
>> as thread local error is allocated on first use) we can have zero
>> fields in a copy as a result. In particular code field can be set
>> to VIR_ERR_OK.
>>
>> In some places (qemu monitor, qemu agent and qemu migaration code
>> for example) we use copy result as a flag and this leads to bugs.
>>
>> Let's set OOM-like error in copy in case of virLastErrorObject failures.
>>
>> Signed-off-by: Nikolay Shirokovskiy 
>> ---
>>
>> Changes from v1:
>> - check @to
>> - set OMM error instead of using virErrorGenericFailure
>>
>>  src/util/virerror.c | 16 +++-
>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/util/virerror.c b/src/util/virerror.c
>> index f198f27..737ea92 100644
>> --- a/src/util/virerror.c
>> +++ b/src/util/virerror.c
>> @@ -366,19 +366,25 @@ virSetError(virErrorPtr newerr)
>>   *
>>   * One will need to free the result with virResetError()
>>   *
>> - * Returns 0 if no error was found and the error code otherwise and -1 in 
>> case
>> - * of parameter error.
>> + * Returns error code or -1 in case of parameter error.
>>   */
>>  int
>>  virCopyLastError(virErrorPtr to)
>>  {
>>  virErrorPtr err = virLastErrorObject();
>> +
>> +if (!to)
>> +return -1;
>> +
>>  /* We can't guarantee caller has initialized it to zero */
>>  memset(to, 0, sizeof(*to));
>> -if (err)
>> +if (err) {
>>  virCopyError(err, to);
>> -else
>> -virResetError(to);
>> +} else {
>> +to->code = VIR_ERR_NO_MEMORY;
>> +to->domain = VIR_FROM_NONE;
>> +to->level = VIR_ERR_ERROR;
> 
> Should we do a VIR_FREE(to->message); so that nothing that was there
> before somehow remains... I don't think either agent or monitor> "lastError" 
> is reset until Dispose time.

Won't hurt but probably will not be used by monitor or agent. If thread
error is not allocated message is NULL upon return, after error is allocated we 
never
hit this OOM branch anymore. Of course hypotetical client can bring @to
with message already set so this a bit future proof. 

I think then we can leave reset and then set these 3 fields.

Nikolay

> 
> Beyond that, I think this works for this edge/odd/bad case...
> 
> Reviewed-by: John Ferlan 
> 
> John
> 
>> +}
>>  return to->code;
>>  }
>>  
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [RFC PATCHv2 00/10] x86 RDT Cache Monitoring Technology (CMT)

2018-07-16 Thread Martin Kletzander

On Mon, Jul 09, 2018 at 03:00:48PM +0800, Wang Huaqiang wrote:


This is the V2 of RFC and the POC source code for introducing x86 RDT CMT
feature, thanks Martin Kletzander for his review and constructive
suggestion for V1.

This series is trying to provide the similar functions of the perf event
based CMT, MBMT and MBML features in reporting cache occupancy, total
memory bandwidth utilization and local memory bandwidth utilization
information in livirt. Firstly we focus on cmt.

x86 RDT Cache Monitoring Technology (CMT) provides a medthod to track the
cache occupancy information per CPU thread. We are leveraging the
implementation of kernel resctrl filesystem and create our patches on top
of that.

Describing the functionality from a high level:

1. Extend the output of 'domstats' and report CMT inforamtion.

Comparing with perf event based CMT implementation in libvirt, this series
extends the output of command 'domstat' and reports cache occupancy
information like these:

[root@dl-c200 libvirt]# virsh domstats vm3 --cpu-resource
Domain: 'vm3'
 cpu.cacheoccupancy.vcpus_2.value=4415488
 cpu.cacheoccupancy.vcpus_2.vcpus=2
 cpu.cacheoccupancy.vcpus_1.value=7839744
 cpu.cacheoccupancy.vcpus_1.vcpus=1
 cpu.cacheoccupancy.vcpus_0,3.value=53796864
 cpu.cacheoccupancy.vcpus_0,3.vcpus=0,3

The vcpus have been arragned into three monitoring groups, these three
groups cover vcpu 1, vcpu 2 and vcpus 0,3 respectively. Take an example,
the 'cpu.cacheoccupancy.vcpus_0,3.value' reports the cache occupancy
information for vcpu 0 and vcpu 3, the 'cpu.cacheoccupancy.vcpus_0,3.vcpus'
represents the vcpu group information.

To address Martin's suggestion "beware as 1-4 is something else than 1,4 so
you need to differentiate that.", the content of 'vcpus'
(cpu.cacheoccupancy..vcpus=xxx) has been specially processed, if
vcpus is a continous range, e.g. 0-2, then the output of
cpu.cacheoccupancy.vcpus_0-2.vcpus will be like
'cpu.cacheoccupancy.vcpus_0-2.vcpus=0,1,2'
instead of
'cpu.cacheoccupancy.vcpus_0-2.vcpus=0-2'.
Please note that 'vcpus_0-2' is a name of this monitoring group, could be
specified any other word from the XML configuration file or lively changed
with the command introduced in following part.



One small nit according to the naming (but it shouldn't block any reviewers from
reviewing, just keep this in mind for next version for example) is that this is
still inconsistent.  The way domstats are structured when there is something
like an array could shed some light into this.  What you suggested is really
kind of hard to parse (although looks better).  What would you say to something
like this:

 cpu.cacheoccupancy.count = 3
 cpu.cacheoccupancy.0.value=4415488
 cpu.cacheoccupancy.0.vcpus=2
 cpu.cacheoccupancy.0.name=vcpus_2
 cpu.cacheoccupancy.1.value=7839744
 cpu.cacheoccupancy.1.vcpus=1
 cpu.cacheoccupancy.1.name=vcpus_1
 cpu.cacheoccupancy.2.value=53796864
 cpu.cacheoccupancy.2.vcpus=0,3
 cpu.cacheoccupancy.2.name=0,3

Other than that I didn't go through all the patches now, sorry.

signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCHv2 00/10] x86 RDT Cache Monitoring Technology (CMT)

2018-07-16 Thread Huaqiang,Wang

Hi,

Regarding the output of CMT monitoring result, which will be
listed in the result of command 'domstats', I'd like to make it changed
by adding a field of 'cache block id' for indicating cache occupancy of
each cache block. So the CMT related message for every cache
monitoring group would be: (see in-lined update for a real example)
"
cpu.cacheoccupancy..vcpus = 
cpu.cacheoccupancy...value
 = 


"
I'd like to hear your voice regarding this RFC.

On 2018年07月09日 15:00, Wang Huaqiang wrote:

This is the V2 of RFC and the POC source code for introducing x86 RDT CMT
feature, thanks Martin Kletzander for his review and constructive
suggestion for V1.

This series is trying to provide the similar functions of the perf event
based CMT, MBMT and MBML features in reporting cache occupancy, total
memory bandwidth utilization and local memory bandwidth utilization
information in livirt. Firstly we focus on cmt.

x86 RDT Cache Monitoring Technology (CMT) provides a medthod to track the
cache occupancy information per CPU thread. We are leveraging the
implementation of kernel resctrl filesystem and create our patches on top
of that.

Describing the functionality from a high level:

1. Extend the output of 'domstats' and report CMT inforamtion.

Comparing with perf event based CMT implementation in libvirt, this series
extends the output of command 'domstat' and reports cache occupancy
information like these:

[root@dl-c200 libvirt]# virsh domstats vm3 --cpu-resource
Domain: 'vm3'
   cpu.cacheoccupancy.vcpus_2.value=4415488
   cpu.cacheoccupancy.vcpus_2.vcpus=2
   cpu.cacheoccupancy.vcpus_1.value=7839744
   cpu.cacheoccupancy.vcpus_1.vcpus=1
   cpu.cacheoccupancy.vcpus_0,3.value=53796864
   cpu.cacheoccupancy.vcpus_0,3.vcpus=0,3


Since kernel resctrlfs outputs cache occupancy information for
each cache block id, I'd like to adhere to resctrlfs's arrangement
and dumping cache occupancy information for each cache block
in the result of 'domstats'. The output of 'domstats' would be like
these:


[root@dl-c200 libvirt]# virsh domstats vm3 --cpu-resource
Domain: 'vm3'
  cpu.cacheoccupancy.vcpus_2.vcpus=2
  cpu.cacheoccupancy.vcpus_2.1.value=27832
  cpu.cacheoccupancy.vcpus_2.0.value=372186
  cpu.cacheoccupancy.vcpus_1.vcpus=1
  cpu.cacheoccupancy.vcpus_1.1.value=0
  cpu.cacheoccupancy.vcpus_1.0.value=90112
  cpu.cacheoccupancy.vcpus_0,3.vcpus=0,3
  cpu.cacheoccupancy.vcpus_0,3.1.value=90112
  cpu.cacheoccupancy.vcpus_0,3.0.value=540672


So from above message, it is known there is a cpu CMT
resource monitoring group exists in domain with the
group name 'vcpus_2'.
'cpu.cacheoccupancy.vcpus_2.vcpus=2' tells us this cpu
resource monitoring group contains one vcpu,  the vcpu 2.
'cpu.cacheoccupancy.vcpus_2.1.value=2387832'
and 'cpu.cacheoccupancy.vcpus_2.0.value=372186' indicate
the cache occupancy information for cache block 1 and
cache block 0 respectively.
You can also get similar information for cpu monitoring
groups 'vcpus_1' and 'vcpus_0,3'.


The vcpus have been arragned into three monitoring groups, these three
groups cover vcpu 1, vcpu 2 and vcpus 0,3 respectively. Take an example,
the 'cpu.cacheoccupancy.vcpus_0,3.value' reports the cache occupancy
information for vcpu 0 and vcpu 3, the 'cpu.cacheoccupancy.vcpus_0,3.vcpus'
represents the vcpu group information.

To address Martin's suggestion "beware as 1-4 is something else than 1,4 so
you need to differentiate that.", the content of 'vcpus'
(cpu.cacheoccupancy..vcpus=xxx) has been specially processed, if
vcpus is a continous range, e.g. 0-2, then the output of
cpu.cacheoccupancy.vcpus_0-2.vcpus will be like
'cpu.cacheoccupancy.vcpus_0-2.vcpus=0,1,2'
instead of
'cpu.cacheoccupancy.vcpus_0-2.vcpus=0-2'.
Please note that 'vcpus_0-2' is a name of this monitoring group, could be
specified any other word from the XML configuration file or lively changed
with the command introduced in following part.

2. A new command 'cpu-resource' for live changing CMT groups.

A virsh tool has been introduced in this series to dynamically create,
destroy monitoring groups as well as showing the existing grouping status.
The general command interface is like this:

[root@dl-c200 libvirt]# virsh help cpu-resource
   NAME
   cpu-resource - get or set hardware CPU RDT monitoring group

   SYNOPSIS
   cpu-resource  [--group-name ] [--vcpulist
   ] [--create] [--destroy] [--live] [--config]
   [--current]

   DESCRIPTION
   Create or destroy CPU resource monitoring group.
   To get current CPU resource monitoring group status:
   virsh # cpu-resource [domain]

   OPTIONS
   [--domain]   domain name, id or uuid
   --group-name   group name to manipulate
   --vcpulist   ids of vcpus to manipulate
   --create Create CPU resctrl monitoring group for
   functions such as monitoring cache occupancy
   --destroy

Re: [libvirt] [PATCH v3] qemu: fix broken autostart symlink after renaming domain.

2018-07-16 Thread Julio Faracco
Changelog:
V1: The patch was automatically rejected due to some tabs
automatically added by my VIM editor.
V2: Tabs changed by spaces.
V3: Rebasing the patch considering the Erik's suggestions.

--
Julio Cesar Faracco
Em ter, 17 de jul de 2018 às 00:52, Julio Faracco
 escreveu:
>
> If a domain is configured to start on boot, it has a symlink to the
> domain definition inside the autostart directory. If you rename this
> domain, the definition is renamed too. The symlink need to be pointed to
> this renamed file. This commit recreates the symlink after renaming the
> XML file.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1594985
>
> Signed-off-by: Julio Faracco 
> ---
>  src/qemu/qemu_driver.c | 42 ++
>  1 file changed, 42 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 8fae46370e..6bbea324b9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20914,6 +20914,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>  char *old_dom_name = NULL;
>  char *new_dom_cfg_file = NULL;
>  char *old_dom_cfg_file = NULL;
> +char *new_dom_autostart_link = NULL;
> +char *old_dom_autostart_link = NULL;
>
>  virCheckFlags(0, ret);
>
> @@ -20934,6 +20936,22 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>   vm->def->name)))
>  goto cleanup;
>
> +if (vm->autostart) {
> +if (!(new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
> +  new_dom_name)) ||
> +!(old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
> +  vm->def->name)))
> +goto cleanup;
> +
> +if (virFileIsLink(old_dom_autostart_link) &&
> +unlink(old_dom_autostart_link) < 0) {
> +virReportSystemError(errno,
> + _("Failed to delete symlink '%s'"),
> + old_dom_autostart_link);
> +goto cleanup;
> +}
> +}
> +
>  event_old = virDomainEventLifecycleNewFromObj(vm,
>  VIR_DOMAIN_EVENT_UNDEFINED,
>  
> VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
> @@ -20946,6 +20964,16 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>  if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
>  goto rollback;
>
> +
> +if (vm->autostart) {
> +if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
> +virReportSystemError(errno,
> + _("Failed to create symlink '%s to '%s'"),
> + new_dom_autostart_link, new_dom_cfg_file);
> +goto rollback;
> +}
> +}
> +
>  if (virFileExists(old_dom_cfg_file) &&
>  unlink(old_dom_cfg_file) < 0) {
>  virReportSystemError(errno,
> @@ -20960,6 +20988,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>  ret = 0;
>
>   cleanup:
> +VIR_FREE(old_dom_autostart_link);
> +VIR_FREE(new_dom_autostart_link);
>  VIR_FREE(old_dom_cfg_file);
>  VIR_FREE(new_dom_cfg_file);
>  VIR_FREE(old_dom_name);
> @@ -20979,6 +21009,18 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>  if (virFileExists(new_dom_cfg_file))
>  unlink(new_dom_cfg_file);
>
> +if (vm->autostart) {
> +if (virFileExists(new_dom_autostart_link))
> +unlink(new_dom_autostart_link);
> +
> +if (!virFileExists(old_dom_autostart_link) &&
> +symlink(old_dom_cfg_file, old_dom_autostart_link) < 0) {
> +virReportSystemError(errno,
> + _("Failed to create symlink '%s to '%s'"),
> + old_dom_autostart_link, old_dom_cfg_file);
> +}
> +}
> +
>  goto cleanup;
>  }
>
> --
> 2.17.1
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3] qemu: fix broken autostart symlink after renaming domain.

2018-07-16 Thread Julio Faracco
If a domain is configured to start on boot, it has a symlink to the
domain definition inside the autostart directory. If you rename this
domain, the definition is renamed too. The symlink need to be pointed to
this renamed file. This commit recreates the symlink after renaming the
XML file.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1594985

Signed-off-by: Julio Faracco 
---
 src/qemu/qemu_driver.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8fae46370e..6bbea324b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20914,6 +20914,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
 char *old_dom_name = NULL;
 char *new_dom_cfg_file = NULL;
 char *old_dom_cfg_file = NULL;
+char *new_dom_autostart_link = NULL;
+char *old_dom_autostart_link = NULL;
 
 virCheckFlags(0, ret);
 
@@ -20934,6 +20936,22 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
  vm->def->name)))
 goto cleanup;
 
+if (vm->autostart) {
+if (!(new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
+  new_dom_name)) ||
+!(old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir,
+  vm->def->name)))
+goto cleanup;
+
+if (virFileIsLink(old_dom_autostart_link) &&
+unlink(old_dom_autostart_link) < 0) {
+virReportSystemError(errno,
+ _("Failed to delete symlink '%s'"),
+ old_dom_autostart_link);
+goto cleanup;
+}
+}
+
 event_old = virDomainEventLifecycleNewFromObj(vm,
 VIR_DOMAIN_EVENT_UNDEFINED,
 
VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
@@ -20946,6 +20964,16 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
 if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
 goto rollback;
 
+
+if (vm->autostart) {
+if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
+virReportSystemError(errno,
+ _("Failed to create symlink '%s to '%s'"),
+ new_dom_autostart_link, new_dom_cfg_file);
+goto rollback;
+}
+}
+
 if (virFileExists(old_dom_cfg_file) &&
 unlink(old_dom_cfg_file) < 0) {
 virReportSystemError(errno,
@@ -20960,6 +20988,8 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
 ret = 0;
 
  cleanup:
+VIR_FREE(old_dom_autostart_link);
+VIR_FREE(new_dom_autostart_link);
 VIR_FREE(old_dom_cfg_file);
 VIR_FREE(new_dom_cfg_file);
 VIR_FREE(old_dom_name);
@@ -20979,6 +21009,18 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
 if (virFileExists(new_dom_cfg_file))
 unlink(new_dom_cfg_file);
 
+if (vm->autostart) {
+if (virFileExists(new_dom_autostart_link))
+unlink(new_dom_autostart_link);
+
+if (!virFileExists(old_dom_autostart_link) &&
+symlink(old_dom_cfg_file, old_dom_autostart_link) < 0) {
+virReportSystemError(errno,
+ _("Failed to create symlink '%s to '%s'"),
+ old_dom_autostart_link, old_dom_cfg_file);
+}
+}
+
 goto cleanup;
 }
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2] util: set OOM in virCopyLastError if error is not set

2018-07-16 Thread John Ferlan



On 07/02/2018 07:33 AM, Nikolay Shirokovskiy wrote:
> virCopyLastError is intended to be used after last error is set.
> However due to virLastErrorObject failures (very unlikely through
> as thread local error is allocated on first use) we can have zero
> fields in a copy as a result. In particular code field can be set
> to VIR_ERR_OK.
> 
> In some places (qemu monitor, qemu agent and qemu migaration code
> for example) we use copy result as a flag and this leads to bugs.
> 
> Let's set OOM-like error in copy in case of virLastErrorObject failures.
> 
> Signed-off-by: Nikolay Shirokovskiy 
> ---
> 
> Changes from v1:
> - check @to
> - set OMM error instead of using virErrorGenericFailure
> 
>  src/util/virerror.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index f198f27..737ea92 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -366,19 +366,25 @@ virSetError(virErrorPtr newerr)
>   *
>   * One will need to free the result with virResetError()
>   *
> - * Returns 0 if no error was found and the error code otherwise and -1 in 
> case
> - * of parameter error.
> + * Returns error code or -1 in case of parameter error.
>   */
>  int
>  virCopyLastError(virErrorPtr to)
>  {
>  virErrorPtr err = virLastErrorObject();
> +
> +if (!to)
> +return -1;
> +
>  /* We can't guarantee caller has initialized it to zero */
>  memset(to, 0, sizeof(*to));
> -if (err)
> +if (err) {
>  virCopyError(err, to);
> -else
> -virResetError(to);
> +} else {
> +to->code = VIR_ERR_NO_MEMORY;
> +to->domain = VIR_FROM_NONE;
> +to->level = VIR_ERR_ERROR;

Should we do a VIR_FREE(to->message); so that nothing that was there
before somehow remains... I don't think either agent or monitor
"lastError" is reset until Dispose time.

Beyond that, I think this works for this edge/odd/bad case...

Reviewed-by: John Ferlan 

John

> +}
>  return to->code;
>  }
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v3 1/1] qemu: Add entry for balloon stat stat-disk-caches

2018-07-16 Thread John Ferlan


On 07/02/2018 07:19 AM, Tomáš Golembiovský wrote:
> QEMU commit bf1e7140e adds reporting of new balloon statistic to QEMU
> 2.12. Value represents the amount of memory that can be quickly
> reclaimed without additional I/O. Let's add that too.
> 
> Signed-off-by: Tomáš Golembiovský 
> ---
>  include/libvirt/libvirt-domain.h | 9 -
>  src/libvirt-domain.c | 3 +++
>  src/qemu/qemu_driver.c   | 1 +
>  src/qemu/qemu_monitor_json.c | 2 ++
>  tools/virsh-domain-monitor.c | 2 ++
>  tools/virsh.pod  | 5 +
>  6 files changed, 21 insertions(+), 1 deletion(-)
> 

Reviewed-by: John Ferlan 
(and pushed)

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 2/3] qemu: Check for existing address when cold attach device

2018-07-16 Thread John Ferlan
Now that virDomainDefCompatibleDevice accepts an action,
let's use that to generically determine whether a to be
cold plugged device with a defined  would conflict
with any existing address for the domain config. This
replaces the RNG check in qemuDomainAttachDeviceConfig.

This type of check would need to be made before a device
is inserted into its domain device list (disk, rng, hostdev,
etc.). This type of check cannot be done during the post
parse processing because the check would conflict with
itself as the device would be placed onto the its device
list prior to post parse.

By comparison this is similar to the validation phase
checks in virDomainDefCheckDuplicateDriveAddresses that
occur during define/startup processing, but are not run
during config attach of a live guest.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 22 ++
 src/qemu/qemu_driver.c |  7 ---
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 82df8012af..fd5bc10c82 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28270,11 +28270,25 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
  virDomainDeviceAction action,
  bool live)
 {
+int target_bus = -1;
 virDomainCompatibleDeviceData data = {
 .newInfo = virDomainDeviceGetInfo(dev),
 .oldInfo = NULL,
 };
 
+if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
+target_bus = dev->data.disk->bus;
+} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
+   dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+virDomainHostdevSubsysPtr subsys = &dev->data.hostdev->source.subsys;
+
+/* If using 'scsi' or 'scsi_host', then the device can be placed
+ * on the same bus as a , so account for that */
+if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI ||
+subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST)
+target_bus = VIR_DOMAIN_DISK_BUS_SCSI;
+}
+
 if (oldDev)
 data.oldInfo = virDomainDeviceGetInfo(oldDev);
 
@@ -28288,6 +28302,14 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
 return -1;
 }
 
+if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH && !live && data.newInfo &&
+data.newInfo->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+virDomainDefHasDeviceAddress(def, target_bus, data.newInfo)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("a device with the same address already exists"));
+return -1;
+}
+
 if (!virDomainDefHasUSB(def) &&
 def->os.type != VIR_DOMAIN_OSTYPE_EXE &&
 virDomainDeviceIsUSB(dev)) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f91d463ae..7d519c0714 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8081,13 +8081,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 break;
 
 case VIR_DOMAIN_DEVICE_RNG:
-if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
-virDomainDefHasDeviceAddress(vmdef, -1, &dev->data.rng->info)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("a device with the same address already exists 
"));
-return -1;
-}
-
 if (VIR_APPEND_ELEMENT(vmdef->rngs, vmdef->nrngs, dev->data.rng) < 0)
 return -1;
 dev->data.rng = NULL;
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 1/3] conf: Add @target_bus to virDomainDefHasDeviceAddress

2018-07-16 Thread John Ferlan
Add the @target_bus argument which will allow a caller to
pass the virDomainDiskBus onto which the @info ()
would be placed. This will allow logic to provide the bus for
cold plugged devices to determine whether the about to
be added device  is already present on the @bus.

Just passing the @info isn't sufficient since, for example,
ADDRESS_TYPE_DRIVE is used for both SCSI and IDE 's
as well as 'scsi' and 'scsi_host' 's.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 32 
 src/conf/domain_conf.h |  3 ++-
 src/qemu/qemu_driver.c |  2 +-
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7396616eda..82df8012af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3688,13 +3688,31 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
 }
 
 
+struct virDomainDefHasDeviceAddressIteratorData {
+int target_bus; /* virDomainDiskBus or -1 */
+virDomainDeviceInfoPtr info;
+};
+
 static int
 virDomainDefHasDeviceAddressIterator(virDomainDefPtr def ATTRIBUTE_UNUSED,
- virDomainDeviceDefPtr dev 
ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev,
  virDomainDeviceInfoPtr info,
  void *opaque)
 {
-virDomainDeviceInfoPtr needle = opaque;
+struct virDomainDefHasDeviceAddressIteratorData *data = opaque;
+int target_bus = data->target_bus;
+virDomainDeviceInfoPtr needle = data->info;
+
+/* If the target_bus of the about to be cold plugged device needs
+ * to be checked and the currently to be matched device is a disk,
+ * then compare it's target bus against the new device. If they don't
+ * match, then no need to compare. For disks this ensures addresses
+ * using drive won't erroneously match if one is IDE and another is SCSI.
+ * Likewise, for SCSI hostdev's this ensures the new hostdev doesn't
+ * erroneously match an IDE for the address comparison. */
+if (target_bus != -1 && dev->type == VIR_DOMAIN_DEVICE_DISK &&
+dev->data.disk->bus != target_bus)
+return 0;
 
 /* break iteration if the info was found */
 if (virDomainDeviceInfoAddressIsEqual(info, needle))
@@ -3933,12 +3951,18 @@ virDomainDeviceInfoIterate(virDomainDefPtr def,
 
 bool
 virDomainDefHasDeviceAddress(virDomainDefPtr def,
+ int target_bus,
  virDomainDeviceInfoPtr info)
 {
+struct virDomainDefHasDeviceAddressIteratorData data = {
+.target_bus = target_bus,
+.info = info,
+};
+
 if (virDomainDeviceInfoIterateInternal(def,

virDomainDefHasDeviceAddressIterator,
true,
-   info) < 0)
+   &data) < 0)
 return true;
 
 return false;
@@ -17508,7 +17532,7 @@ virDomainMemoryInsert(virDomainDefPtr def,
 int id = def->nmems;
 
 if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
-virDomainDefHasDeviceAddress(def, &mem->info)) {
+virDomainDefHasDeviceAddress(def, -1, &mem->info)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Domain already contains a device with the same "
  "address"));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0f10e242fd..82231161c6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2912,8 +2912,9 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
void *opaque);
 
 bool virDomainDefHasDeviceAddress(virDomainDefPtr def,
+  int target_bus,
   virDomainDeviceInfoPtr info)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
 
 void virDomainDefFree(virDomainDefPtr vm);
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8fae46370e..5f91d463ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8082,7 +8082,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 
 case VIR_DOMAIN_DEVICE_RNG:
 if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
-virDomainDefHasDeviceAddress(vmdef, &dev->data.rng->info)) {
+virDomainDefHasDeviceAddress(vmdef, -1, &dev->data.rng->info)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("a device with the same address already exists 
"));
 return -1;
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach

2018-07-16 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1559867

When attaching a device to the domain we need to be sure
to use the correct domain definition (vm->def or vm->newDef)
when calling virDomainDeviceDefParse because the post parse
processing algorithms that may assign an address for the
device will use whatever domain definition was passed in.

Additionally, some devices (SCSI hostdev and SCSI disk) use
algorithms that rely on knowing what already exists of the
other type when generating the new device's address. Using
the wrong VM definition could result in duplicated addresses.

In the case of the bz, two hostdev's with no domain address
provided were added to the running domain's config only.
However, the parsing algorithm used the live domain in
order to figure out the host device address resulting in
the same address being used and a subsequent start failing
due to duplicate address.

Fix this by separating the checks/code into CONFIG and LIVE
processing using the correct definition for each block and
performing cleanup for both options as necessary.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c | 52 +++---
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7d519c0714..3dbd5c62d2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8473,7 +8473,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 {
 virDomainDefPtr vmdef = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
-virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
+virDomainDeviceDefPtr devConf = NULL;
+virDomainDeviceDefPtr devLive = NULL;
 int ret = -1;
 virCapsPtr caps = NULL;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -8487,49 +8488,43 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
-dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
- caps, driver->xmlopt,
- parse_flags);
-if (dev == NULL)
-goto cleanup;
-
-if (virDomainDeviceValidateAliasForHotplug(vm, dev, flags) < 0)
-goto cleanup;
-
-if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
-flags & VIR_DOMAIN_AFFECT_LIVE) {
-/* If we are affecting both CONFIG and LIVE
- * create a deep copy of device as adding
- * to CONFIG takes one instance.
- */
-dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
-if (!dev_copy)
-goto cleanup;
-}
-
+/* The config and live post processing address auto-generation algorithms
+ * rely on the correct vm->def or vm->newDef being passed, so call the
+ * device parse based on which definition is in use */
 if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-/* Make a copy for updated domain. */
 vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
 if (!vmdef)
 goto cleanup;
 
-if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
+if (!(devConf = virDomainDeviceDefParse(xml, vmdef, caps,
+driver->xmlopt, parse_flags)))
+goto cleanup;
+
+if (virDomainDefCompatibleDevice(vmdef, devConf, NULL,
  VIR_DOMAIN_DEVICE_ACTION_ATTACH,
  false) < 0)
 goto cleanup;
-if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
+
+if ((ret = qemuDomainAttachDeviceConfig(vmdef, devConf, caps,
 parse_flags,
 driver->xmlopt)) < 0)
 goto cleanup;
 }
 
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
+if (!(devLive = virDomainDeviceDefParse(xml, vm->def, caps,
+driver->xmlopt, parse_flags)))
+goto cleanup;
+
+if (virDomainDeviceValidateAliasForHotplug(vm, devLive, flags) < 0)
+goto cleanup;
+
+if (virDomainDefCompatibleDevice(vm->def, devLive, NULL,
  VIR_DOMAIN_DEVICE_ACTION_ATTACH,
  true) < 0)
 goto cleanup;
 
-if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0)
+if ((ret = qemuDomainAttachDeviceLive(vm, devLive, driver)) < 0)
 goto cleanup;
 /*
  * update domain status forcibly because the domain status may be
@@ -8553,9 +8548,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 
  cleanup:
 virDomainDefFree(vmdef);
-if (dev != dev_copy)
-virDomainDeviceDefFree(dev_copy);
-virDomainDeviceDefFree(dev);
+virDomainDevice

[libvirt] [PATCH v3 0/3] Alter qemu live/cold device attach algorithm

2018-07-16 Thread John Ferlan
v2: https://www.redhat.com/archives/libvir-list/2018-July/msg00361.html

Differences to v2:

Patch1: NEW - As a result of code review the suggestion was to utilize
the virDomainDefCompatibleDevice in order to make the check more
generic to include 's which were also afflicted with the
same problem that I was trying to solve with the former patch1
just for 's. However, this led me down into the abyss of
more changes since 's have multiple 
target bus types (IDE and SCSI). That means we need to have a
mechanism to pass the target bus along so that a SCSI drive
address doesn't inadvertently match an IDE drive address. All
that is complicated by the way virDomainDefHasDeviceAddress
iterates through all the device lists.  Whether there are more
similar devices I'm assuming will fall out of code review.

Patch2: This moves the virDomainDefHasDeviceAddress into the more
common config checking virDomainDefCompatibleDevice method,
but now needs to also account for the disk bus issue.

This theoretically could be combined with Patch1, but keeping
them separate I would hope makes for simpler code review. I
could also move code out of virDomainDefHasDeviceAddressIterator
into patch2, but if just felt better in patch1.

Patch3: No changes were made (amazingly so).

In the end quite a bit more complicated

John Ferlan (3):
  conf: Add @target_bus to virDomainDefHasDeviceAddress
  qemu: Check for existing address when cold attach device
  qemu: Use the correct vm def on cold attach

 src/conf/domain_conf.c | 54 +++---
 src/conf/domain_conf.h |  3 ++-
 src/qemu/qemu_driver.c | 59 --
 3 files changed, 75 insertions(+), 41 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] AUTHORS: Add Katerina Koukiou

2018-07-16 Thread Ján Tomko
$ git log --format=oneline --committer=kkoukiou | wc -l
3

Also set up the mailmap entry to avoid duplicates in the
generated authors file.

Signed-off-by: Ján Tomko 
---
 .mailmap   | 1 +
 AUTHORS.in | 1 +
 2 files changed, 2 insertions(+)

diff --git a/.mailmap b/.mailmap
index 913395b89c..311a10a8e9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -42,6 +42,7 @@
  
  
  
+ 
 
 # Name consolidation:
 # Preferred author spelling 
diff --git a/AUTHORS.in b/AUTHORS.in
index 62afa56e27..fbac54c48d 100644
--- a/AUTHORS.in
+++ b/AUTHORS.in
@@ -25,6 +25,7 @@ Ján Tomko 
 Jim Fehlig 
 Jiří Denemark 
 John Ferlan 
+Katerina Koukiou 
 Laine Stump 
 Mark McLoughlin 
 Martin Kletzander 
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 0/2] tests: Add/Update QEMU caps data for 2.11+ on x86_64

2018-07-16 Thread Erik Skultety
Since v1:
- regenerated domaincapsschemadata to satisfy the test suite

Erik Skultety (2):
  tests: Add capabilities data for QEMU 2.11 x86_64
  tests: Update capabilities data for QEMU 2.12+ x86_64

 tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml  |37 +-
 .../caps_2.11.0.x86_64.replies | 19416 +++
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |   961 +
 .../caps_2.12.0.x86_64.replies |   539 +-
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   183 +-
 .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies |   219 +-
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |57 +-
 tests/qemucapabilitiestest.c   | 1 +
 8 files changed, 20807 insertions(+), 606 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml

--
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 2/2] tests: Update capabilities data for QEMU 2.12+ x86_64

2018-07-16 Thread Erik Skultety
The original capabilities didn't include a patched kernel for spectre
and meltdown, SPICE gl support and had xen support enabled which we
already have dropped.

Signed-off-by: Erik Skultety 
---
 tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml  |  37 +-
 .../caps_2.12.0.x86_64.replies | 539 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 183 ++-
 .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies | 219 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  57 +--
 5 files changed, 429 insertions(+), 606 deletions(-)

diff --git a/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml 
b/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
index 7a1be4c093..334e4bebaf 100644
--- a/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml
@@ -23,18 +23,15 @@
   
 
 
-  Haswell-noTSX
+  Skylake-Client-IBRS
   Intel
-  
   
-  
-  
+  
   
-  
   
-  
+  
+  
   
-  
   
 
 
@@ -51,13 +48,13 @@
   core2duo
   athlon
   Westmere
-  Westmere-IBRS
+  Westmere-IBRS
   Skylake-Server
   Skylake-Server-IBRS
-  Skylake-Client
-  Skylake-Client-IBRS
+  Skylake-Client
+  Skylake-Client-IBRS
   SandyBridge
-  SandyBridge-IBRS
+  SandyBridge-IBRS
   Penryn
   Opteron_G5
   Opteron_G4
@@ -65,20 +62,20 @@
   Opteron_G2
   Opteron_G1
   Nehalem
-  Nehalem-IBRS
+  Nehalem-IBRS
   IvyBridge
-  IvyBridge-IBRS
-  Haswell
+  IvyBridge-IBRS
+  Haswell
   Haswell-noTSX
-  Haswell-noTSX-IBRS
-  Haswell-IBRS
+  Haswell-noTSX-IBRS
+  Haswell-IBRS
   EPYC
   EPYC-IBPB
   Conroe
-  Broadwell
-  Broadwell-noTSX
-  Broadwell-noTSX-IBRS
-  Broadwell-IBRS
+  Broadwell
+  Broadwell-noTSX
+  Broadwell-noTSX-IBRS
+  Broadwell-IBRS
   486
 
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
index 6f37e4301e..334afaa61b 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
@@ -17,11 +17,11 @@
 {
   "return": {
 "qemu": {
-  "micro": 90,
-  "minor": 11,
+  "micro": 0,
+  "minor": 12,
   "major": 2
 },
-"package": "v2.12.0-rc0"
+"package": "v2.12.0"
   },
   "id": "libvirt-2"
 }
@@ -549,7 +549,7 @@
 
 {
   "return": {
-"fd": 19,
+"fd": 17,
 "fdset-id": 0
   },
   "id": "libvirt-5"
@@ -3307,13 +3307,26 @@
 
 {
   "return": [
+{
+  "name": "min_io_size",
+  "type": "uint16"
+},
+{
+  "name": "removable",
+  "description": "on/off",
+  "type": "bool"
+},
+{
+  "name": "channel",
+  "type": "uint32"
+},
 {
   "name": "serial",
   "type": "str"
 },
 {
-  "name": "port_index",
-  "type": "uint16"
+  "name": "lun",
+  "type": "uint32"
 },
 {
   "name": "dpofua",
@@ -3321,99 +3334,90 @@
   "type": "bool"
 },
 {
-  "name": "bootindex",
-  "type": "int32"
+  "name": "ver",
+  "type": "str"
+},
+{
+  "name": "scsi-id",
+  "type": "uint32"
 },
 {
   "name": "logical_block_size",
   "description": "A power of two between 512 and 32768",
   "type": "uint16"
 },
-{
-  "name": "discard_granularity",
-  "type": "uint32"
-},
-{
-  "name": "lun",
-  "type": "uint32"
-},
-{
-  "name": "max_unmap_size",
-  "type": "uint64"
-},
 {
   "name": "drive",
   "description": "Node name or ID of a block device to use as a backend",
   "type": "str"
 },
+{
+  "name": "scsi_version",
+  "type": "int32"
+},
+{
+  "name": "werror",
+  "description": "Error handling policy, report/ignore/enospc/stop/auto",
+  "type": "BlockdevOnError"
+},
+{
+  "name": "discard_granularity",
+  "type": "uint32"
+},
 {
   "name": "port_wwn",
   "type": "uint64"
 },
 {
-  "name": "write-cache",
-  "description": "on/off/auto",
-  "type": "OnOffAuto"
+  "name": "max_unmap_size",
+  "type": "uint64"
+},
+{
+  "name": "rerror",
+  "description": "Error handling policy, report/ignore/enospc/stop/auto",
+  "type": "BlockdevOnError"
+},
+{
+  "name": "max_io_size",
+  "type": "uint64"
+},
+{
+  "name": "wwn",
+  "type": "uint64"
 },
 {
   "name": "share-rw",
   "type": "bool"
 },
-{
-  "name": "opt_io_size",
-  "type": "uint32"
-},
-{
-  "name": "min_io_size",
-  "type": "uint16"
-},
 {
   "name": "product",
   "type": "str"
 },
-{
-  "name": "scsi-id",
-  "type": "uint32"
-},
-{
-  "nam

Re: [libvirt] [PATCH 0/2] tests: Add/Update QEMU caps data for 2.11+ on x86_64

2018-07-16 Thread Erik Skultety
On Mon, Jul 16, 2018 at 04:51:42PM +0200, Erik Skultety wrote:
> Erik Skultety (2):
>   tests: Add capabilities data for QEMU 2.11 x86_64
>   tests: Update capabilities data for QEMU 2.12+ x86_64
>
>  .../caps_2.11.0.x86_64.replies | 19416 
> +++
>  tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |   961 +
>  .../caps_2.12.0.x86_64.replies |   539 +-
>  tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   183 +-
>  .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies |   219 +-
>  tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |57 +-
>  tests/qemucapabilitiestest.c   | 1 +
>  7 files changed, 20790 insertions(+), 586 deletions(-)
>  create mode 100644 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.replies
>  create mode 100644 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml
>
> --
> 2.14.4
>

NACK, domaincapstest fails because I forgot regenerate domaincapsschemadata.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 0/2] tests: Add/Update QEMU caps data for 2.11+ on x86_64

2018-07-16 Thread Erik Skultety
Erik Skultety (2):
  tests: Add capabilities data for QEMU 2.11 x86_64
  tests: Update capabilities data for QEMU 2.12+ x86_64

 .../caps_2.11.0.x86_64.replies | 19416 +++
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |   961 +
 .../caps_2.12.0.x86_64.replies |   539 +-
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   183 +-
 .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies |   219 +-
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |57 +-
 tests/qemucapabilitiestest.c   | 1 +
 7 files changed, 20790 insertions(+), 586 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml

--
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/2] tests: Update capabilities data for QEMU 2.12+ x86_64

2018-07-16 Thread Erik Skultety
The original capabilities didn't include a patched kernel for spectre
and meltdown, SPICE gl support and had xen support enabled which we
already have dropped.

Signed-off-by: Erik Skultety 
---
 .../caps_2.12.0.x86_64.replies | 539 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 183 ++-
 .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies | 219 -
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  57 +--
 4 files changed, 412 insertions(+), 586 deletions(-)

diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
index 6f37e4301e..334afaa61b 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies
@@ -17,11 +17,11 @@
 {
   "return": {
 "qemu": {
-  "micro": 90,
-  "minor": 11,
+  "micro": 0,
+  "minor": 12,
   "major": 2
 },
-"package": "v2.12.0-rc0"
+"package": "v2.12.0"
   },
   "id": "libvirt-2"
 }
@@ -549,7 +549,7 @@
 
 {
   "return": {
-"fd": 19,
+"fd": 17,
 "fdset-id": 0
   },
   "id": "libvirt-5"
@@ -3307,13 +3307,26 @@
 
 {
   "return": [
+{
+  "name": "min_io_size",
+  "type": "uint16"
+},
+{
+  "name": "removable",
+  "description": "on/off",
+  "type": "bool"
+},
+{
+  "name": "channel",
+  "type": "uint32"
+},
 {
   "name": "serial",
   "type": "str"
 },
 {
-  "name": "port_index",
-  "type": "uint16"
+  "name": "lun",
+  "type": "uint32"
 },
 {
   "name": "dpofua",
@@ -3321,99 +3334,90 @@
   "type": "bool"
 },
 {
-  "name": "bootindex",
-  "type": "int32"
+  "name": "ver",
+  "type": "str"
+},
+{
+  "name": "scsi-id",
+  "type": "uint32"
 },
 {
   "name": "logical_block_size",
   "description": "A power of two between 512 and 32768",
   "type": "uint16"
 },
-{
-  "name": "discard_granularity",
-  "type": "uint32"
-},
-{
-  "name": "lun",
-  "type": "uint32"
-},
-{
-  "name": "max_unmap_size",
-  "type": "uint64"
-},
 {
   "name": "drive",
   "description": "Node name or ID of a block device to use as a backend",
   "type": "str"
 },
+{
+  "name": "scsi_version",
+  "type": "int32"
+},
+{
+  "name": "werror",
+  "description": "Error handling policy, report/ignore/enospc/stop/auto",
+  "type": "BlockdevOnError"
+},
+{
+  "name": "discard_granularity",
+  "type": "uint32"
+},
 {
   "name": "port_wwn",
   "type": "uint64"
 },
 {
-  "name": "write-cache",
-  "description": "on/off/auto",
-  "type": "OnOffAuto"
+  "name": "max_unmap_size",
+  "type": "uint64"
+},
+{
+  "name": "rerror",
+  "description": "Error handling policy, report/ignore/enospc/stop/auto",
+  "type": "BlockdevOnError"
+},
+{
+  "name": "max_io_size",
+  "type": "uint64"
+},
+{
+  "name": "wwn",
+  "type": "uint64"
 },
 {
   "name": "share-rw",
   "type": "bool"
 },
-{
-  "name": "opt_io_size",
-  "type": "uint32"
-},
-{
-  "name": "min_io_size",
-  "type": "uint16"
-},
 {
   "name": "product",
   "type": "str"
 },
-{
-  "name": "scsi-id",
-  "type": "uint32"
-},
-{
-  "name": "channel",
-  "type": "uint32"
-},
 {
   "name": "vendor",
   "type": "str"
 },
-{
-  "name": "wwn",
-  "type": "uint64"
-},
-{
-  "name": "werror",
-  "description": "Error handling policy, report/ignore/enospc/stop/auto",
-  "type": "BlockdevOnError"
-},
-{
-  "name": "removable",
-  "description": "on/off",
-  "type": "bool"
-},
-{
-  "name": "rerror",
-  "description": "Error handling policy, report/ignore/enospc/stop/auto",
-  "type": "BlockdevOnError"
-},
-{
-  "name": "ver",
-  "type": "str"
-},
 {
   "name": "physical_block_size",
   "description": "A power of two between 512 and 32768",
   "type": "uint16"
 },
 {
-  "name": "max_io_size",
-  "type": "uint64"
+  "name": "port_index",
+  "type": "uint16"
+},
+{
+  "name": "bootindex",
+  "type": "int32"
+},
+{
+  "name": "write-cache",
+  "description": "on/off/auto",
+  "type": "OnOffAuto"
+},
+{
+  "name": "opt_io_size",
+  "type": "uint32"
 }
   ],
   "id": "libvirt-19"
@@ -4323,6 +4327,11 @@
   "name": "vectors",
   "type": "uint32"
 },
+{
+  "name": "iommu_platform",
+  "description": "on/off",
+  "type": "bool"
+},
 {
   "name": "x-pcie-extcap-init",
   "description": "on/off",
@@ -4340,26 +4349,31 @@
   "name": "x-ignore-backend-f

Re: [libvirt] [PATCH v3 7/8] qemu: command: Enable formatting vfio-pci.display option onto cmdline

2018-07-16 Thread Erik Skultety
On Fri, Jul 13, 2018 at 03:06:05PM +0200, Ján Tomko wrote:
> On Wed, Jul 11, 2018 at 03:58:27PM +0200, Erik Skultety wrote:
> > Since QEMU 2.12, QEMU understands a new vfio-pci device option 'display'
> > which can be used to turn on display capabilities on vgpu-enabled
> > mediated devices, IOW emulated GPU devices like QXL will no longer be
> > needed with vgpu-enable mdevs.
> > QEMU defaults to 'auto' for the 'display' attribute, which is not
> > foolproof, so we need to play it safe here and explicitly format
> > display='off' if this attribute wasn't provided in the XML explicitly.
> >
> > Signed-off-by: Erik Skultety 
> > ---
> > src/qemu/qemu_command.c| 25 -
> > .../hostdev-mdev-display-missing-graphics.xml  | 35 ++
> > .../hostdev-mdev-display-spice-egl-headless.args   | 32 +
> > .../hostdev-mdev-display-spice-egl-headless.xml| 40 
> > +
> > .../hostdev-mdev-display-spice-opengl.args | 31 
> > .../hostdev-mdev-display-spice-opengl.xml  | 41 
> > ++
> > .../hostdev-mdev-display-vnc-egl-headless.args | 32 +
> > .../hostdev-mdev-display-vnc-egl-headless.xml  | 40 
> > +
> > .../qemuxml2argvdata/hostdev-mdev-display-vnc.args | 31 
> > .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml  | 39 
> > tests/qemuxml2argvtest.c   | 31 
> > 11 files changed, 376 insertions(+), 1 deletion(-)
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.args
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.args
> > create mode 100644 
> > tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml
> > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.args
> > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 48e224cabc..5f6b340f8f 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -5207,6 +5207,26 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef 
> > *def,
> > virBufferAdd(&buf, dev_str, -1);
> > virBufferAsprintf(&buf, ",id=%s,sysfsdev=%s", dev->info->alias, 
> > mdevPath);
> >
> > +/* QEMU 2.12 added support for vfio-pci display type, we need to 
> > perform
> > + * some additional checks here */
> > +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY)) {
> > +if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT) {
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +   _("display property of device vfio-pci is "
> > + "not supported by this version of QEMU"));
> > +goto cleanup;
> > +}
>
> qemuCaps checks belong to *Validate functions.

Well, we still need to come up with the default value, but only in case QEMU
has the cap and we already agreed upon not doing this in postparse, otherwise
we'd format something into the offline config even if there was no input from
the user, so I'd like to keep it this way, otherwise the capability check would
have to be at 2 places instead of 1.

>
> > +} else {
> > + /* we default to 'display=off', since QEMU defaults to 'auto' 
> > which is
> > +  * unreliable and we don't want to risk any breakages */
> > +if (mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT)
> > +mdevsrc->display = VIR_TRISTATE_SWITCH_OFF;
> > +}
> > +
> > +if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT)
> > +virBufferAsprintf(&buf, ",display=%s",
> > +  virTristateSwitchTypeToString(mdevsrc->display));
> > +
> > if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)
> > goto cleanup;
> >
>
> > @@ -5424,7 +5444,9 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
> >
> > /* MDEV */
> > if (virHostdevIsMdevDevice(hostdev)) {
> > -switch ((virMediatedDeviceModelType) subsys->u.mdev.model) {
> > +virDomainHostdevSubsysMediatedDevPtr mdevsrc = &subsys->u.mdev;
> > +
> > +switch ((virMediatedDeviceModelType) mdevsrc->model) {
> > case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
> > if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > @@ -5432,6 +5454,7 @@ qemuBuildHostdevCommandLine(virComma

Re: [libvirt] [Qemu-devel] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

2018-07-16 Thread Markus Armbruster
Cornelia Huck  writes:

> On Mon, 16 Jul 2018 09:54:12 +0200
> Thomas Huth  wrote:
>
>> On 16.07.2018 09:32, Markus Armbruster wrote:
>> > Libvirt developers would like to be copied on patches to qemu-doc
>> > appendix "Deprecated features".  Do them the favor.
>> > 
>> > Signed-off-by: Markus Armbruster 
>> > ---
>> >  MAINTAINERS | 4 
>> >  1 file changed, 4 insertions(+)
>> > 
>> > diff --git a/MAINTAINERS b/MAINTAINERS
>> > index 20eef3cb61..666e936812 100644
>> > --- a/MAINTAINERS
>> > +++ b/MAINTAINERS
>> > @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
>> >  S: Odd Fixes
>> >  F: docs/devel/build-system.txt
>> >  
>> > +Incompatible changes
>> > +R: libvir-list@redhat.com
>> > +F: qemu-deprecated.texi  
>> 
>> Should we have a maintainer for the file, too? (I guess not, because
>> deprecation patches should go through the specific subsystems...)
>
> I don't think adding a maintainer makes sense for this file.

This MAINTAINERS entry doesn't declare maintainers, only reviewers.

We can change that if a maintainer steps up.

>> And what about a "S:" line?
>
> I don't think that makes too much sense, either.
>
> If anything, qemu-deprecated.texi should be in a category 'maintained
> by everybody', i.e. qemu-devel. Just like qemu-doc.texi, which does not
> have an entry in MAINTAINERS at all.

Just like all the other files that lack a maintainer.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: qemu_driver: Fix setting global_period cputune element

2018-07-16 Thread Ján Tomko

qemu_driver: is not necessary in the commit summary

On Mon, Jul 16, 2018 at 03:08:44PM +0200, Katerina Koukiou wrote:

When VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD is matched "cputune.global_period"
should be updated and not "cputune.period".

Signed-off-by: Katerina Koukiou 


We have a public bug filed for this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1600427


---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

And pushed.

Jano


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 33/37] lxc: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each lxc C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 lxc.go | 20 ++---
 lxc_wrapper.go | 58 +++---
 lxc_wrapper.h  | 26 +-
 3 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/lxc.go b/lxc.go
index ec685b8..56ab28a 100644
--- a/lxc.go
+++ b/lxc.go
@@ -47,9 +47,10 @@ import (
 func (d *Domain) LxcOpenNamespace(flags uint32) ([]os.File, error) {
var cfdlist *C.int
 
-   ret := C.virDomainLxcOpenNamespace(d.ptr, &cfdlist, C.uint(flags))
+   var err C.virError
+   ret := C.virDomainLxcOpenNamespaceWrapper(d.ptr, &cfdlist, 
C.uint(flags), &err)
if ret == -1 {
-   return []os.File{}, GetLastError()
+   return []os.File{}, makeError(&err)
}
fdlist := make([]os.File, ret)
for i := 0; i < int(ret); i++ {
@@ -69,9 +70,10 @@ func (d *Domain) LxcEnterNamespace(fdlist []os.File, flags 
uint32) ([]os.File, e
cfdlist[i] = C.int(fdlist[i].Fd())
}
 
-   ret := C.virDomainLxcEnterNamespace(d.ptr, C.uint(len(fdlist)), 
&cfdlist[0], &ncoldfdlist, &coldfdlist, C.uint(flags))
+   var err C.virError
+   ret := C.virDomainLxcEnterNamespaceWrapper(d.ptr, C.uint(len(fdlist)), 
&cfdlist[0], &ncoldfdlist, &coldfdlist, C.uint(flags), &err)
if ret == -1 {
-   return []os.File{}, GetLastError()
+   return []os.File{}, makeError(&err)
}
oldfdlist := make([]os.File, ncoldfdlist)
for i := 0; i < int(ncoldfdlist); i++ {
@@ -119,9 +121,10 @@ func DomainLxcEnterSecurityLabel(model *NodeSecurityModel, 
label *SecurityLabel,
clabel.enforcing = 0
}
 
-   ret := C.virDomainLxcEnterSecurityLabel(&cmodel, &clabel, &coldlabel, 
C.uint(flags))
+   var err C.virError
+   ret := C.virDomainLxcEnterSecurityLabelWrapper(&cmodel, &clabel, 
&coldlabel, C.uint(flags), &err)
if ret == -1 {
-   return nil, GetLastError()
+   return nil, makeError(&err)
}
 
var oldlabel SecurityLabel
@@ -141,10 +144,11 @@ func (d *Domain) DomainLxcEnterCGroup(flags uint32) error 
{
return GetNotImplementedError("virDomainLxcEnterCGroup")
}
 
-   ret := C.virDomainLxcEnterCGroupWrapper(d.ptr, C.uint(flags))
+   var err C.virError
+   ret := C.virDomainLxcEnterCGroupWrapper(d.ptr, C.uint(flags), &err)
 
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
 
return nil
diff --git a/lxc_wrapper.go b/lxc_wrapper.go
index 0968870..fa3d910 100644
--- a/lxc_wrapper.go
+++ b/lxc_wrapper.go
@@ -34,16 +34,68 @@ package libvirt
 #include 
 #include "lxc_wrapper.h"
 
-int virDomainLxcEnterCGroupWrapper(virDomainPtr domain,
- unsigned int flags)
+int
+virDomainLxcEnterCGroupWrapper(virDomainPtr domain,
+   unsigned int flags,
+   virErrorPtr err)
 {
 #if LIBVIR_VERSION_NUMBER < 200
 assert(0); // Caller should have checked version
 #else
-return virDomainLxcEnterCGroup(domain, flags);
+int ret = virDomainLxcEnterCGroup(domain, flags);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
 #endif
 }
 
 
+int
+virDomainLxcEnterNamespaceWrapper(virDomainPtr domain,
+  unsigned int nfdlist,
+  int *fdlist,
+  unsigned int *noldfdlist,
+  int **oldfdlist,
+  unsigned int flags,
+  virErrorPtr err)
+{
+int ret = virDomainLxcEnterNamespace(domain, nfdlist, fdlist, noldfdlist, 
oldfdlist, flags);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
+}
+
+
+int
+virDomainLxcEnterSecurityLabelWrapper(virSecurityModelPtr model,
+  virSecurityLabelPtr label,
+  virSecurityLabelPtr oldlabel,
+  unsigned int flags,
+  virErrorPtr err)
+{
+int ret = virDomainLxcEnterSecurityLabel(model, label, oldlabel, flags);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
+}
+
+
+int
+virDomainLxcOpenNamespaceWrapper(virDomainPtr domain,
+ int **fdlist,
+ unsigned int flags,
+ virErrorPtr err)
+{
+int ret = virDomainLxcOpenNamespace(domain, fdlist, flags);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
+}
+
+
 */
 import "C"
diff --git a

[libvirt] [go PATCH 34/37] events: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each events C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 domain_events.go   | 131 -
 domain_events_wrapper.go   |  33 +++--
 domain_events_wrapper.h|   7 +-
 events.go  |  40 +++---
 events_wrapper.go  |  83 ++---
 events_wrapper.h   |  35 ++---
 network_events.go  |  11 ++-
 network_events_wrapper.go  |  26 +--
 network_events_wrapper.h   |  13 +---
 node_device_events.go  |  16 ++--
 node_device_events_wrapper.go  |  31 ++--
 node_device_events_wrapper.h   |   6 +-
 secret_events.go   |  16 ++--
 secret_events_wrapper.go   |  27 +--
 secret_events_wrapper.h|   6 +-
 storage_pool_events.go |  16 ++--
 storage_pool_events_wrapper.go |  31 ++--
 storage_pool_events_wrapper.h  |   6 +-
 18 files changed, 382 insertions(+), 152 deletions(-)

diff --git a/domain_events.go b/domain_events.go
index 68dc301..fe46c5e 100644
--- a/domain_events.go
+++ b/domain_events.go
@@ -966,13 +966,14 @@ func (c *Connect) DomainEventLifecycleRegister(dom 
*Domain, callback DomainEvent
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainEventRegisterAnyWrapper(c.ptr, cdom,
C.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
C.virConnectDomainEventGenericCallback(callbackPtr),
-   C.long(goCallBackId))
+   C.long(goCallBackId), &err)
if ret == -1 {
freeCallbackId(goCallBackId)
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return int(ret), nil
 }
@@ -985,13 +986,14 @@ func (c *Connect) DomainEventRebootRegister(dom *Domain, 
callback DomainEventGen
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainEventRegisterAnyWrapper(c.ptr, cdom,
C.VIR_DOMAIN_EVENT_ID_REBOOT,
C.virConnectDomainEventGenericCallback(callbackPtr),
-   C.long(goCallBackId))
+   C.long(goCallBackId), &err)
if ret == -1 {
freeCallbackId(goCallBackId)
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return int(ret), nil
 }
@@ -1004,13 +1006,14 @@ func (c *Connect) DomainEventRTCChangeRegister(dom 
*Domain, callback DomainEvent
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainEventRegisterAnyWrapper(c.ptr, cdom,
C.VIR_DOMAIN_EVENT_ID_RTC_CHANGE,
C.virConnectDomainEventGenericCallback(callbackPtr),
-   C.long(goCallBackId))
+   C.long(goCallBackId), &err)
if ret == -1 {
freeCallbackId(goCallBackId)
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return int(ret), nil
 }
@@ -1023,13 +1026,14 @@ func (c *Connect) DomainEventWatchdogRegister(dom 
*Domain, callback DomainEventW
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainEventRegisterAnyWrapper(c.ptr, cdom,
C.VIR_DOMAIN_EVENT_ID_WATCHDOG,
C.virConnectDomainEventGenericCallback(callbackPtr),
-   C.long(goCallBackId))
+   C.long(goCallBackId), &err)
if ret == -1 {
freeCallbackId(goCallBackId)
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return int(ret), nil
 }
@@ -1042,13 +1046,14 @@ func (c *Connect) DomainEventIOErrorRegister(dom 
*Domain, callback DomainEventIO
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainEventRegisterAnyWrapper(c.ptr, cdom,
C.VIR_DOMAIN_EVENT_ID_IO_ERROR,
C.virConnectDomainEventGenericCallback(callbackPtr),
-   C.long(goCallBackId))
+   C.long(goCallBackId), &err)
if ret == -1 {
freeCallbackId(goCallBackId)
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return int(ret), nil
 }
@@ -1061,13 +1066,14 @@ func (c *Connect) DomainEventGraphicsRegister(dom 
*Domain, callback DomainEventG
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainEventRegisterAnyWrapper(c.ptr, cdom,
C.VIR_DOMAIN_EVENT_ID_GRAPHICS,
C.virConnectDomainEventGenericCallback(callbackPtr),
-   C.long(goCallBack

[libvirt] [go PATCH 24/37] nwfilter: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each nwfilter C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 nwfilter.go | 35 ++
 nwfilter_wrapper.go | 88 +
 nwfilter_wrapper.h  | 33 +
 3 files changed, 142 insertions(+), 14 deletions(-)

diff --git a/nwfilter.go b/nwfilter.go
index 441fcca..0b55c41 100644
--- a/nwfilter.go
+++ b/nwfilter.go
@@ -43,36 +43,40 @@ type NWFilter struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterFree
 func (f *NWFilter) Free() error {
-   ret := C.virNWFilterFree(f.ptr)
+   var err C.virError
+   ret := C.virNWFilterFreeWrapper(f.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterRef
 func (c *NWFilter) Ref() error {
-   ret := C.virNWFilterRef(c.ptr)
+   var err C.virError
+   ret := C.virNWFilterRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterGetName
 func (f *NWFilter) GetName() (string, error) {
-   name := C.virNWFilterGetName(f.ptr)
+   var err C.virError
+   name := C.virNWFilterGetNameWrapper(f.ptr, &err)
if name == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(name), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterUndefine
 func (f *NWFilter) Undefine() error {
-   result := C.virNWFilterUndefine(f.ptr)
+   var err C.virError
+   result := C.virNWFilterUndefineWrapper(f.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -81,9 +85,10 @@ func (f *NWFilter) Undefine() error {
 func (f *NWFilter) GetUUID() ([]byte, error) {
var cUuid [C.VIR_UUID_BUFLEN](byte)
cuidPtr := unsafe.Pointer(&cUuid)
-   result := C.virNWFilterGetUUID(f.ptr, (*C.uchar)(cuidPtr))
+   var err C.virError
+   result := C.virNWFilterGetUUIDWrapper(f.ptr, (*C.uchar)(cuidPtr), &err)
if result != 0 {
-   return []byte{}, GetLastError()
+   return []byte{}, makeError(&err)
}
return C.GoBytes(cuidPtr, C.VIR_UUID_BUFLEN), nil
 }
@@ -92,18 +97,20 @@ func (f *NWFilter) GetUUID() ([]byte, error) {
 func (f *NWFilter) GetUUIDString() (string, error) {
var cUuid [C.VIR_UUID_STRING_BUFLEN](C.char)
cuidPtr := unsafe.Pointer(&cUuid)
-   result := C.virNWFilterGetUUIDString(f.ptr, (*C.char)(cuidPtr))
+   var err C.virError
+   result := C.virNWFilterGetUUIDStringWrapper(f.ptr, (*C.char)(cuidPtr), 
&err)
if result != 0 {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString((*C.char)(cuidPtr)), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterGetXMLDesc
 func (f *NWFilter) GetXMLDesc(flags uint32) (string, error) {
-   result := C.virNWFilterGetXMLDesc(f.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virNWFilterGetXMLDescWrapper(f.ptr, C.uint(flags), &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
xml := C.GoString(result)
C.free(unsafe.Pointer(result))
diff --git a/nwfilter_wrapper.go b/nwfilter_wrapper.go
index 78d0487..809e527 100644
--- a/nwfilter_wrapper.go
+++ b/nwfilter_wrapper.go
@@ -30,5 +30,93 @@ package libvirt
 #include 
 #include "nwfilter_wrapper.h"
 
+
+int
+virNWFilterFreeWrapper(virNWFilterPtr nwfilter,
+   virErrorPtr err)
+{
+int ret = virNWFilterFree(nwfilter);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
+}
+
+
+const char *
+virNWFilterGetNameWrapper(virNWFilterPtr nwfilter,
+  virErrorPtr err)
+{
+const char * ret = virNWFilterGetName(nwfilter);
+if (!ret) {
+virCopyLastError(err);
+}
+return ret;
+}
+
+
+int
+virNWFilterGetUUIDWrapper(virNWFilterPtr nwfilter,
+  unsigned char *uuid,
+  virErrorPtr err)
+{
+int ret = virNWFilterGetUUID(nwfilter, uuid);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
+}
+
+
+int
+virNWFilterGetUUIDStringWrapper(virNWFilterPtr nwfilter,
+char *buf,
+virErrorPtr err)
+{
+int ret = vi

[libvirt] [go PATCH 37/37] connect: add missing references on domain object in stats records

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 connect.go | 4 
 1 file changed, 4 insertions(+)

diff --git a/connect.go b/connect.go
index 34a96ed..8cc7cc7 100644
--- a/connect.go
+++ b/connect.go
@@ -2915,6 +2915,10 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
stats[i] = domstats
}
 
+   for i := 0; i < len(stats); i++ {
+   C.virDomainRef(stats[i].Domain.ptr)
+   }
+
return stats, nil
 }
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 36/37] error: make GetNotImplementedError private

2018-07-16 Thread Daniel P . Berrangé
The GetNotImplementedError() method is only for internal use so should
not have a public name.

Signed-off-by: Daniel P. Berrangé 
---
 connect.go | 24 +-
 domain.go  | 56 +-
 error.go   |  2 +-
 lxc.go |  2 +-
 network.go |  2 +-
 network_events.go  |  4 +--
 node_device_events.go  |  4 +--
 nwfilter_binding.go| 12 -
 qemu.go|  4 +--
 secret_events.go   |  6 ++---
 storage_pool_events.go |  6 ++---
 storage_volume.go  |  2 +-
 stream.go  | 10 
 13 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/connect.go b/connect.go
index 29e4fb0..34a96ed 100644
--- a/connect.go
+++ b/connect.go
@@ -857,7 +857,7 @@ func (c *Connect) DomainDefineXML(xmlConfig string) 
(*Domain, error) {
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDefineXMLFlags
 func (c *Connect) DomainDefineXMLFlags(xmlConfig string, flags 
DomainDefineFlags) (*Domain, error) {
if C.LIBVIR_VERSION_NUMBER < 1002012 {
-   return nil, GetNotImplementedError("virDomainDefineXMLFlags")
+   return nil, makeNotImplementedError("virDomainDefineXMLFlags")
}
cXml := C.CString(string(xmlConfig))
defer C.free(unsafe.Pointer(cXml))
@@ -1266,7 +1266,7 @@ func (c *Connect) LookupStoragePoolByUUID(uuid []byte) 
(*StoragePool, error) {
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolLookupByTargetPath
 func (c *Connect) LookupStoragePoolByTargetPath(path string) (*StoragePool, 
error) {
if C.LIBVIR_VERSION_NUMBER < 4001000 {
-   return nil, 
GetNotImplementedError("virStoragePoolLookupByTargetPath")
+   return nil, 
makeNotImplementedError("virStoragePoolLookupByTargetPath")
}
cPath := C.CString(path)
defer C.free(unsafe.Pointer(cPath))
@@ -1335,7 +1335,7 @@ func (c *Connect) LookupNWFilterByUUID(uuid []byte) 
(*NWFilter, error) {
 // See also 
https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterBindingLookupByPortDev
 func (c *Connect) LookupNWFilterBindingByPortDev(name string) 
(*NWFilterBinding, error) {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
-   return nil, 
GetNotImplementedError("virNWFilterBindingLookupByPortDev")
+   return nil, 
makeNotImplementedError("virNWFilterBindingLookupByPortDev")
}
cName := C.CString(name)
defer C.free(unsafe.Pointer(cName))
@@ -1555,7 +1555,7 @@ func (c *Connect) ListAllNWFilters(flags uint32) 
([]NWFilter, error) {
 func (c *Connect) ListAllNWFilterBindings(flags uint32) ([]NWFilterBinding, 
error) {
var cList *C.virNWFilterBindingPtr
if C.LIBVIR_VERSION_NUMBER < 4005000 {
-   return []NWFilterBinding{}, 
GetNotImplementedError("virConnectListAllNWFilterBindings")
+   return []NWFilterBinding{}, 
makeNotImplementedError("virConnectListAllNWFilterBindings")
}
var err C.virError
numNWFilters := C.virConnectListAllNWFilterBindingsWrapper(c.ptr, 
(**C.virNWFilterBindingPtr)(&cList), C.uint(flags), &err)
@@ -1675,7 +1675,7 @@ func (c *Connect) InterfaceChangeRollback(flags uint32) 
error {
 // See also 
https://libvirt.org/html/libvirt-libvirt-host.html#virNodeAllocPages
 func (c *Connect) AllocPages(pageSizes map[int]int64, startCell int, cellCount 
uint, flags NodeAllocPagesFlags) (int, error) {
if C.LIBVIR_VERSION_NUMBER < 1002009 {
-   return 0, GetNotImplementedError("virNodeAllocPages")
+   return 0, makeNotImplementedError("virNodeAllocPages")
}
cpages := make([]C.uint, len(pageSizes))
ccounts := make([]C.ulonglong, len(pageSizes))
@@ -1812,7 +1812,7 @@ func (c *Connect) GetFreeMemory() (uint64, error) {
 // See also 
https://libvirt.org/html/libvirt-libvirt-host.html#virNodeGetFreePages
 func (c *Connect) GetFreePages(pageSizes []uint64, startCell int, maxCells 
uint, flags uint32) ([]uint64, error) {
if C.LIBVIR_VERSION_NUMBER < 1002006 {
-   return []uint64{}, GetNotImplementedError("virNodeGetFreePages")
+   return []uint64{}, 
makeNotImplementedError("virNodeGetFreePages")
}
cpageSizes := make([]C.uint, len(pageSizes))
ccounts := make([]C.ulonglong, len(pageSizes)*int(maxCells))
@@ -2087,7 +2087,7 @@ func (c *Connect) BaselineCPU(xmlCPUs []string, flags 
ConnectBaselineCPUFlags) (
 // See also 
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectBaselineHypervisorCPU
 func (c *Connect) BaselineHypervisorCPU(emulator string, arch string, machine 
string, virttype string, xmlCPUs []string, flags ConnectBaselineCPUFlags) 
(string, error) {
if C.LIBVIR_VERSION_NUMBER < 4004000 {
-   return "", 
GetNotImplementedError("virConnectBaselineHypervisorCPU")
+   return "", 
makeNotImplemen

[libvirt] [go PATCH 29/37] domain snapshot: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each domain snapshot C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 domain_snapshot.go |  62 -
 domain_snapshot_wrapper.go | 181 +
 domain_snapshot_wrapper.h  |  70 ++
 3 files changed, 288 insertions(+), 25 deletions(-)

diff --git a/domain_snapshot.go b/domain_snapshot.go
index 706c1a9..65fbbb5 100644
--- a/domain_snapshot.go
+++ b/domain_snapshot.go
@@ -90,45 +90,50 @@ type DomainSnapshot struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotFree
 func (s *DomainSnapshot) Free() error {
-   ret := C.virDomainSnapshotFree(s.ptr)
+   var err C.virError
+   ret := C.virDomainSnapshotFreeWrapper(s.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotRef
 func (c *DomainSnapshot) Ref() error {
-   ret := C.virDomainSnapshotRef(c.ptr)
+   var err C.virError
+   ret := C.virDomainSnapshotRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotDelete
 func (s *DomainSnapshot) Delete(flags DomainSnapshotDeleteFlags) error {
-   result := C.virDomainSnapshotDelete(s.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virDomainSnapshotDeleteWrapper(s.ptr, C.uint(flags), &err)
if result != 0 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainRevertToSnapshot
 func (s *DomainSnapshot) RevertToSnapshot(flags DomainSnapshotRevertFlags) 
error {
-   result := C.virDomainRevertToSnapshot(s.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virDomainRevertToSnapshotWrapper(s.ptr, C.uint(flags), &err)
if result != 0 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotIsCurrent
 func (s *DomainSnapshot) IsCurrent(flags uint32) (bool, error) {
-   result := C.virDomainSnapshotIsCurrent(s.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virDomainSnapshotIsCurrentWrapper(s.ptr, C.uint(flags), 
&err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
if result == 1 {
return true, nil
@@ -138,9 +143,10 @@ func (s *DomainSnapshot) IsCurrent(flags uint32) (bool, 
error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotHasMetadata
 func (s *DomainSnapshot) HasMetadata(flags uint32) (bool, error) {
-   result := C.virDomainSnapshotHasMetadata(s.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virDomainSnapshotHasMetadataWrapper(s.ptr, C.uint(flags), 
&err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
if result == 1 {
return true, nil
@@ -150,9 +156,10 @@ func (s *DomainSnapshot) HasMetadata(flags uint32) (bool, 
error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotGetXMLDesc
 func (s *DomainSnapshot) GetXMLDesc(flags DomainXMLFlags) (string, error) {
-   result := C.virDomainSnapshotGetXMLDesc(s.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virDomainSnapshotGetXMLDescWrapper(s.ptr, C.uint(flags), 
&err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
xml := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -161,27 +168,30 @@ func (s *DomainSnapshot) GetXMLDesc(flags DomainXMLFlags) 
(string, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotGetName
 func (s *DomainSnapshot) GetName() (string, error) {
-   name := C.virDomainSnapshotGetName(s.ptr)
+   var err C.virError
+   name := C.virDomainSnapshotGetNameWrapper(s.ptr, &err)
if name == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(name), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotGetParent
 func (s *DomainSnapshot) GetParent(flags uint32) (*DomainSnap

[libvirt] [go PATCH 26/37] node device: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each node device C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 node_device.go |  62 ++---
 node_device_wrapper.go | 150 +
 node_device_wrapper.h  |  55 +++
 3 files changed, 242 insertions(+), 25 deletions(-)

diff --git a/node_device.go b/node_device.go
index 0caf98b..474f288 100644
--- a/node_device.go
+++ b/node_device.go
@@ -57,45 +57,50 @@ type NodeDevice struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceFree
 func (n *NodeDevice) Free() error {
-   ret := C.virNodeDeviceFree(n.ptr)
+   var err C.virError
+   ret := C.virNodeDeviceFreeWrapper(n.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceRef
 func (c *NodeDevice) Ref() error {
-   ret := C.virNodeDeviceRef(c.ptr)
+   var err C.virError
+   ret := C.virNodeDeviceRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy
 func (n *NodeDevice) Destroy() error {
-   result := C.virNodeDeviceDestroy(n.ptr)
+   var err C.virError
+   result := C.virNodeDeviceDestroyWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReset
 func (n *NodeDevice) Reset() error {
-   result := C.virNodeDeviceReset(n.ptr)
+   var err C.virError
+   result := C.virNodeDeviceResetWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDettach
 func (n *NodeDevice) Detach() error {
-   result := C.virNodeDeviceDettach(n.ptr)
+   var err C.virError
+   result := C.virNodeDeviceDettachWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -104,36 +109,40 @@ func (n *NodeDevice) Detach() error {
 func (n *NodeDevice) DetachFlags(driverName string, flags uint32) error {
cDriverName := C.CString(driverName)
defer C.free(unsafe.Pointer(cDriverName))
-   result := C.virNodeDeviceDetachFlags(n.ptr, cDriverName, C.uint(flags))
+   var err C.virError
+   result := C.virNodeDeviceDetachFlagsWrapper(n.ptr, cDriverName, 
C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReAttach
 func (n *NodeDevice) ReAttach() error {
-   result := C.virNodeDeviceReAttach(n.ptr)
+   var err C.virError
+   result := C.virNodeDeviceReAttachWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetName
 func (n *NodeDevice) GetName() (string, error) {
-   name := C.virNodeDeviceGetName(n.ptr)
+   var err C.virError
+   name := C.virNodeDeviceGetNameWrapper(n.ptr, &err)
if name == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(name), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetXMLDesc
 func (n *NodeDevice) GetXMLDesc(flags uint32) (string, error) {
-   result := C.virNodeDeviceGetXMLDesc(n.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virNodeDeviceGetXMLDescWrapper(n.ptr, C.uint(flags), &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
xml := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -142,9 +151,10 @@ func (n *NodeDevice) GetXMLDesc(flags uint32) (string, 
error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetParent
 func (n *NodeDevice) GetParent() (string, error) {
-   result := C.virNodeDeviceGetParent(n.ptr)
+   var err C.virError
+   result := C.virNodeDeviceGetParentWrapper(n.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "

[libvirt] [go PATCH 28/37] interface: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each interface C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 interface.go |  45 +---
 interface_wrapper.go | 124 +++
 interface_wrapper.h  |  44 +++
 3 files changed, 195 insertions(+), 18 deletions(-)

diff --git a/interface.go b/interface.go
index bd39ed0..9b6ebb2 100644
--- a/interface.go
+++ b/interface.go
@@ -49,27 +49,30 @@ type Interface struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceCreate
 func (n *Interface) Create(flags uint32) error {
-   result := C.virInterfaceCreate(n.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virInterfaceCreateWrapper(n.ptr, C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDestroy
 func (n *Interface) Destroy(flags uint32) error {
-   result := C.virInterfaceDestroy(n.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virInterfaceDestroyWrapper(n.ptr, C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceIsActive
 func (n *Interface) IsActive() (bool, error) {
-   result := C.virInterfaceIsActive(n.ptr)
+   var err C.virError
+   result := C.virInterfaceIsActiveWrapper(n.ptr, &err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
if result == 1 {
return true, nil
@@ -79,9 +82,10 @@ func (n *Interface) IsActive() (bool, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetMACString
 func (n *Interface) GetMACString() (string, error) {
-   result := C.virInterfaceGetMACString(n.ptr)
+   var err C.virError
+   result := C.virInterfaceGetMACStringWrapper(n.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
mac := C.GoString(result)
return mac, nil
@@ -89,9 +93,10 @@ func (n *Interface) GetMACString() (string, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetName
 func (n *Interface) GetName() (string, error) {
-   result := C.virInterfaceGetName(n.ptr)
+   var err C.virError
+   result := C.virInterfaceGetNameWrapper(n.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
name := C.GoString(result)
return name, nil
@@ -99,9 +104,10 @@ func (n *Interface) GetName() (string, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetXMLDesc
 func (n *Interface) GetXMLDesc(flags InterfaceXMLFlags) (string, error) {
-   result := C.virInterfaceGetXMLDesc(n.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virInterfaceGetXMLDescWrapper(n.ptr, C.uint(flags), &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
xml := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -110,27 +116,30 @@ func (n *Interface) GetXMLDesc(flags InterfaceXMLFlags) 
(string, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceUndefine
 func (n *Interface) Undefine() error {
-   result := C.virInterfaceUndefine(n.ptr)
+   var err C.virError
+   result := C.virInterfaceUndefineWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceFree
 func (n *Interface) Free() error {
-   ret := C.virInterfaceFree(n.ptr)
+   var err C.virError
+   ret := C.virInterfaceFreeWrapper(n.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceRef
 func (c *Interface) Ref() error {
-   ret := C.virInterfaceRef(c.ptr)
+   var err C.virError
+   ret := C.virInterfaceRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
diff --git a/interface_wrapper.go b/interface_wrapper.go
index c9618fb..a33aea9 100644
--- a/int

[libvirt] [go PATCH 16/37] domain: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 domain.go |   1 +
 domain_compat.h   | 141 ---
 domain_compat.go => domain_wrapper.go |   2 +-
 domain_wrapper.h  | 157 ++
 4 files changed, 159 insertions(+), 142 deletions(-)
 rename domain_compat.go => domain_wrapper.go (99%)
 create mode 100644 domain_wrapper.h

diff --git a/domain.go b/domain.go
index 75c3458..7253c67 100644
--- a/domain.go
+++ b/domain.go
@@ -32,6 +32,7 @@ package libvirt
 #include 
 #include 
 #include "domain_compat.h"
+#include "domain_wrapper.h"
 */
 import "C"
 
diff --git a/domain_compat.h b/domain_compat.h
index 3a868fe..c0d93b6 100644
--- a/domain_compat.h
+++ b/domain_compat.h
@@ -68,11 +68,6 @@
 #define VIR_MIGRATE_AUTO_CONVERGE 1 << 13
 #endif
 
-int virDomainCoreDumpWithFormatWrapper(virDomainPtr domain,
- const char *to,
- unsigned int dumpformat,
- unsigned int flags);
-
 
 /* 1.2.5 */
 
@@ -88,27 +83,6 @@ int virDomainCoreDumpWithFormatWrapper(virDomainPtr domain,
 #define VIR_DOMAIN_TIME_SYNC 1 << 0
 #endif
 
-int virDomainGetTimeWrapper(virDomainPtr dom,
-  long long *seconds,
-  unsigned int *nseconds,
-  unsigned int flags);
-
-int virDomainSetTimeWrapper(virDomainPtr dom,
-  long long seconds,
-  unsigned int nseconds,
-  unsigned int flags);
-
-int virDomainFSFreezeWrapper(virDomainPtr dom,
-   const char **mountpoints,
-   unsigned int nmountpoints,
-   unsigned int flags);
-
-int virDomainFSThawWrapper(virDomainPtr dom,
- const char **mountpoints,
- unsigned int nmountpoints,
- unsigned int flags);
-
-
 /* 1.2.6 */
 
 #ifndef VIR_DOMAIN_BLOCK_COMMIT_ACTIVE
@@ -161,17 +135,6 @@ int virDomainFSThawWrapper(virDomainPtr dom,
 #define VIR_DOMAIN_STATS_STATE 1 << 0
 #endif
 
-int virDomainBlockCopyWrapper(virDomainPtr dom, const char *disk,
-const char *destxml,
-virTypedParameterPtr params,
-int nparams,
-unsigned int flags);
-
-int virDomainOpenGraphicsFDWrapper(virDomainPtr dom,
- unsigned int idx,
- unsigned int flags);
-
-
 /* 1.2.9 */
 
 #ifndef VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES
@@ -374,13 +337,6 @@ struct _virDomainFSInfo {
 };
 #endif
 
-void virDomainFSInfoFreeWrapper(virDomainFSInfoPtr info);
-
-int virDomainGetFSInfoWrapper(virDomainPtr dom,
-virDomainFSInfoPtr **info,
-unsigned int flags);
-
-
 /* 1.2.12 */
 
 #ifndef VIR_DOMAIN_DEFINE_VALIDATE
@@ -450,24 +406,6 @@ struct _virDomainInterface {
 };
 #endif
 
-int virDomainInterfaceAddressesWrapper(virDomainPtr dom,
- virDomainInterfacePtr **ifaces,
- unsigned int source,
- unsigned int flags);
-
-void virDomainInterfaceFreeWrapper(virDomainInterfacePtr iface);
-
-void virDomainIOThreadInfoFreeWrapper(virDomainIOThreadInfoPtr info);
-
-int virDomainGetIOThreadInfoWrapper(virDomainPtr domain,
-  virDomainIOThreadInfoPtr **info,
-  unsigned int flags);
-int virDomainPinIOThreadWrapper(virDomainPtr domain,
-  unsigned int iothread_id,
-  unsigned char *cpumap,
-  int maplen,
-  unsigned int flags);
-
 
 /* 1.2.15 */
 
@@ -483,13 +421,6 @@ int virDomainPinIOThreadWrapper(virDomainPtr domain,
 #define VIR_DOMAIN_EVENT_ID_DEVICE_ADDED 19
 #endif
 
-int virDomainAddIOThreadWrapper(virDomainPtr domain,
-  unsigned int iothread_id,
-  unsigned int flags);
-int virDomainDelIOThreadWrapper(virDomainPtr domain,
-  unsigned int iothread_id,
-  unsigned int flags);
-
 
 /* 1.2.16 */
 
@@ -497,11 +428,6 @@ int virDomainDelIOThreadWrapper(virDomainPtr domain,
 #define VIR_DOMAIN_PASSWORD_ENCRYPTED 1 << 0
 #endif
 
-int virDomainSetUserPasswordWrapper(virDomainPtr dom,
-  const char *user,
-  const char *password,
-  unsigned int flags);
-
 
 /* 1.2.17 */
 
@@ -528,10 +454,6 @@ int virDomainSetUserPasswordWrapper(virDomainPtr dom,
 #define VIR_DOMAIN_EVENT_UNDEFINED_RENAMED 1
 #endif
 
-int virDomainRenameWrapper(virDomainPtr dom,
- const char *ne

[libvirt] [go PATCH 32/37] qemu: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each qemu C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 qemu.go | 31 ++-
 qemu_wrapper.go | 79 -
 qemu_wrapper.h  | 33 +
 3 files changed, 115 insertions(+), 28 deletions(-)

diff --git a/qemu.go b/qemu.go
index 3631b00..dd4f258 100644
--- a/qemu.go
+++ b/qemu.go
@@ -81,10 +81,11 @@ func (d *Domain) QemuMonitorCommand(command string, flags 
DomainQemuMonitorComma
var cResult *C.char
cCommand := C.CString(command)
defer C.free(unsafe.Pointer(cCommand))
-   result := C.virDomainQemuMonitorCommand(d.ptr, cCommand, &cResult, 
C.uint(flags))
+   var err C.virError
+   result := C.virDomainQemuMonitorCommandWrapper(d.ptr, cCommand, 
&cResult, C.uint(flags), &err)
 
if result != 0 {
-   return "", GetLastError()
+   return "", makeError(&err)
}
 
rstring := C.GoString(cResult)
@@ -95,10 +96,11 @@ func (d *Domain) QemuMonitorCommand(command string, flags 
DomainQemuMonitorComma
 func (d *Domain) QemuAgentCommand(command string, timeout 
DomainQemuAgentCommandTimeout, flags uint32) (string, error) {
cCommand := C.CString(command)
defer C.free(unsafe.Pointer(cCommand))
-   result := C.virDomainQemuAgentCommand(d.ptr, cCommand, C.int(timeout), 
C.uint(flags))
+   var err C.virError
+   result := C.virDomainQemuAgentCommandWrapper(d.ptr, cCommand, 
C.int(timeout), C.uint(flags), &err)
 
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
 
rstring := C.GoString(result)
@@ -107,10 +109,10 @@ func (d *Domain) QemuAgentCommand(command string, timeout 
DomainQemuAgentCommand
 }
 
 func (c *Connect) DomainQemuAttach(pid uint32, flags uint32) (*Domain, error) {
-
-   ptr := C.virDomainQemuAttach(c.ptr, C.uint(pid), C.uint(flags))
+   var err C.virError
+   ptr := C.virDomainQemuAttachWrapper(c.ptr, C.uint(pid), C.uint(flags), 
&err)
if ptr == nil {
-   return nil, GetLastError()
+   return nil, makeError(&err)
}
return &Domain{ptr: ptr}, nil
 }
@@ -156,19 +158,18 @@ func (c *Connect) DomainQemuMonitorEventRegister(dom 
*Domain, event string, call
defer C.free(unsafe.Pointer(cEvent))
goCallBackId := registerCallbackId(callback)
 
-   callbackPtr := unsafe.Pointer(C.domainQemuMonitorEventCallbackHelper)
var cdom C.virDomainPtr
if dom != nil {
cdom = dom.ptr
}
+   var err C.virError
ret := C.virConnectDomainQemuMonitorEventRegisterWrapper(c.ptr, cdom,
cEvent,
-   C.virConnectDomainQemuMonitorEventCallback(callbackPtr),
C.long(goCallBackId),
-   C.uint(flags))
-   if ret == -1 {
+   C.uint(flags), &err)
+   if ret < 0 {
freeCallbackId(goCallBackId)
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return int(ret), nil
 }
@@ -179,8 +180,10 @@ func (c *Connect) DomainQemuEventDeregister(callbackId 
int) error {
}
 
// Deregister the callback
-   if i := int(C.virConnectDomainQemuMonitorEventDeregisterWrapper(c.ptr, 
C.int(callbackId))); i != 0 {
-   return GetLastError()
+   var err C.virError
+   ret := int(C.virConnectDomainQemuMonitorEventDeregisterWrapper(c.ptr, 
C.int(callbackId), &err))
+   if ret < 0 {
+   return makeError(&err)
}
return nil
 }
diff --git a/qemu_wrapper.go b/qemu_wrapper.go
index 1dda84d..20089d2 100644
--- a/qemu_wrapper.go
+++ b/qemu_wrapper.go
@@ -45,26 +45,89 @@ void domainQemuMonitorEventCallbackHelper(virConnectPtr c, 
virDomainPtr d,
 domainQemuMonitorEventCallback(c, d, event, secs, micros, details, 
(int)(intptr_t)data);
 }
 
-int virConnectDomainQemuMonitorEventRegisterWrapper(virConnectPtr c,  
virDomainPtr d,
-const char *event, 
virConnectDomainQemuMonitorEventCallback cb,
-long goCallbackId, 
unsigned int flags) {
+
+int
+virConnectDomainQemuMonitorEventDeregisterWrapper(virConnectPtr conn,
+  int callbackID,
+  virErrorPtr err)
+{
 #if LIBVIR_VERSION_NUMBER < 1002003
 assert(0); // Caller should have checked version
 #else
-void* id = (void*)goCallbackId;
-return virConnectDomainQemuMonitorEventRegister(c, d, event, cb, id, 
freeGoCallbackHelper, flags);
+int ret = virConnectDomainQemuMonitorEventDeregister(conn, callbackID);
+if (ret < 0) {
+vir

[libvirt] [go PATCH 21/37] storage pool: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each storage pool C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 storage_pool.go | 117 +--
 storage_pool_wrapper.go | 309 
 storage_pool_wrapper.h  | 117 +++
 3 files changed, 496 insertions(+), 47 deletions(-)

diff --git a/storage_pool.go b/storage_pool.go
index a2d1462..9bfcc79 100644
--- a/storage_pool.go
+++ b/storage_pool.go
@@ -105,54 +105,60 @@ type StoragePoolInfo struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolBuild
 func (p *StoragePool) Build(flags StoragePoolBuildFlags) error {
-   result := C.virStoragePoolBuild(p.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virStoragePoolBuildWrapper(p.ptr, C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolCreate
 func (p *StoragePool) Create(flags StoragePoolCreateFlags) error {
-   result := C.virStoragePoolCreate(p.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virStoragePoolCreateWrapper(p.ptr, C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolDelete
 func (p *StoragePool) Delete(flags StoragePoolDeleteFlags) error {
-   result := C.virStoragePoolDelete(p.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virStoragePoolDeleteWrapper(p.ptr, C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolDestroy
 func (p *StoragePool) Destroy() error {
-   result := C.virStoragePoolDestroy(p.ptr)
+   var err C.virError
+   result := C.virStoragePoolDestroyWrapper(p.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolFree
 func (p *StoragePool) Free() error {
-   ret := C.virStoragePoolFree(p.ptr)
+   var err C.virError
+   ret := C.virStoragePoolFreeWrapper(p.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolRef
 func (c *StoragePool) Ref() error {
-   ret := C.virStoragePoolRef(c.ptr)
+   var err C.virError
+   ret := C.virStoragePoolRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -160,9 +166,10 @@ func (c *StoragePool) Ref() error {
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolGetAutostart
 func (p *StoragePool) GetAutostart() (bool, error) {
var out C.int
-   result := C.virStoragePoolGetAutostart(p.ptr, 
(*C.int)(unsafe.Pointer(&out)))
+   var err C.virError
+   result := C.virStoragePoolGetAutostartWrapper(p.ptr, 
(*C.int)(unsafe.Pointer(&out)), &err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
switch out {
case 1:
@@ -175,9 +182,10 @@ func (p *StoragePool) GetAutostart() (bool, error) {
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolGetInfo
 func (p *StoragePool) GetInfo() (*StoragePoolInfo, error) {
var cinfo C.virStoragePoolInfo
-   result := C.virStoragePoolGetInfo(p.ptr, &cinfo)
+   var err C.virError
+   result := C.virStoragePoolGetInfoWrapper(p.ptr, &cinfo, &err)
if result == -1 {
-   return nil, GetLastError()
+   return nil, makeError(&err)
}
return &StoragePoolInfo{
State:  StoragePoolState(cinfo.state),
@@ -189,9 +197,10 @@ func (p *StoragePool) GetInfo() (*StoragePoolInfo, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolGetName
 func (p *StoragePool) GetName() (string, error) {
-   name := C.virStoragePoolGetName(p.ptr)
+   var err C.virError
+   name := C.virStoragePoolGetNameWrapper(p.ptr, &err)
if name == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(name), nil
 }
@@ -200,9 +209,10 @@ func (p *Stor

[libvirt] [go PATCH 11/37] stream: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 stream_compat.go  | 69 ---
 stream_compat.h   | 13 -
 stream_wrapper.go | 34 +++
 stream_wrapper.h  | 12 +
 4 files changed, 46 insertions(+), 82 deletions(-)
 delete mode 100644 stream_compat.go

diff --git a/stream_compat.go b/stream_compat.go
deleted file mode 100644
index d718ae1..000
--- a/stream_compat.go
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * This file is part of the libvirt-go project
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * Copyright (C) 2017 Red Hat, Inc.
- *
- */
-
-package libvirt
-
-/*
-#cgo pkg-config: libvirt
-#include 
-#include 
-#include "stream_compat.h"
-
-int virStreamRecvFlagsWrapper(virStreamPtr st,
-char *data,
-size_t nbytes,
-unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 3004000
-assert(0); // Caller should have checked version
-#else
-return virStreamRecvFlags(st, data, nbytes, flags);
-#endif
-}
-
-int virStreamSendHoleWrapper(virStreamPtr st,
-   long long length,
-   unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 3004000
-assert(0); // Caller should have checked version
-#else
-return virStreamSendHole(st, length, flags);
-#endif
-}
-
-int virStreamRecvHoleWrapper(virStreamPtr st,
-   long long *length,
-   unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 3004000
-assert(0); // Caller should have checked version
-#else
-return virStreamRecvHole(st, length, flags);
-#endif
-}
-
-*/
-import "C"
diff --git a/stream_compat.h b/stream_compat.h
index 9db2184..92befd5 100644
--- a/stream_compat.h
+++ b/stream_compat.h
@@ -33,17 +33,4 @@
 #define VIR_STREAM_RECV_STOP_AT_HOLE (1 << 0)
 #endif
 
-int virStreamRecvFlagsWrapper(virStreamPtr st,
-char *data,
-size_t nbytes,
-unsigned int flags);
-
-int virStreamSendHoleWrapper(virStreamPtr st,
-   long long length,
-   unsigned int flags);
-
-int virStreamRecvHoleWrapper(virStreamPtr,
-   long long *length,
-   unsigned int flags);
-
 #endif /* LIBVIRT_GO_STREAM_COMPAT_H__ */
diff --git a/stream_wrapper.go b/stream_wrapper.go
index 4e1c2c9..419bb41 100644
--- a/stream_wrapper.go
+++ b/stream_wrapper.go
@@ -128,5 +128,39 @@ int virStreamEventAddCallbackWrapper(virStreamPtr st, int 
events, int callbackID
 return virStreamEventAddCallback(st, events, streamEventCallbackHelper, 
(void *)(intptr_t)callbackID, NULL);
 }
 
+int virStreamRecvFlagsWrapper(virStreamPtr st,
+char *data,
+size_t nbytes,
+unsigned int flags)
+{
+#if LIBVIR_VERSION_NUMBER < 3004000
+assert(0); // Caller should have checked version
+#else
+return virStreamRecvFlags(st, data, nbytes, flags);
+#endif
+}
+
+int virStreamSendHoleWrapper(virStreamPtr st,
+   long long length,
+   unsigned int flags)
+{
+#if LIBVIR_VERSION_NUMBER < 3004000
+assert(0); // Caller should have checked version
+#else
+return virStreamSendHole(st, length, flags);
+#endif
+}
+
+int virStreamRecvHoleWrapper(virStreamPtr st,
+   long long *length,
+   unsigned int flags)
+{
+#if LIBVIR_VERSION_NUMBER < 3004000
+assert(0); // Caller should have checked version
+#else
+return virStreamRecvHole(st, length, flags);
+#endif
+}
+
 */
 import "C"
diff --git a/stream_wrapper.h b/stream_wrapper.h
index c064423..cfa6c37 100644
--- a/stream_wrapper.h
+++ b/stream_wrapper.h
@@ -33,5 +33,17 @@ int virStreamSparseSendAllWrapper(virStreamPtr st, int 
callback

[libvirt] [go PATCH 25/37] nwfilter binding: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each nwfilter binding C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 nwfilter_binding.go | 30 +++---
 nwfilter_binding_wrapper.go | 63 +
 nwfilter_binding_wrapper.h  | 27 ++--
 3 files changed, 85 insertions(+), 35 deletions(-)

diff --git a/nwfilter_binding.go b/nwfilter_binding.go
index 722303b..5999d3e 100644
--- a/nwfilter_binding.go
+++ b/nwfilter_binding.go
@@ -45,9 +45,10 @@ func (f *NWFilterBinding) Free() error {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return GetNotImplementedError("virNWFilterBindingFree")
}
-   ret := C.virNWFilterBindingFreeWrapper(f.ptr)
+   var err C.virError
+   ret := C.virNWFilterBindingFreeWrapper(f.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -57,9 +58,10 @@ func (c *NWFilterBinding) Ref() error {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return GetNotImplementedError("virNWFilterBindingRef")
}
-   ret := C.virNWFilterBindingRefWrapper(c.ptr)
+   var err C.virError
+   ret := C.virNWFilterBindingRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -69,9 +71,10 @@ func (f *NWFilterBinding) Delete() error {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return GetNotImplementedError("virNWFilterBindingDelete")
}
-   result := C.virNWFilterBindingDeleteWrapper(f.ptr)
+   var err C.virError
+   result := C.virNWFilterBindingDeleteWrapper(f.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -81,9 +84,10 @@ func (f *NWFilterBinding) GetPortDev() (string, error) {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return "", 
GetNotImplementedError("virNWFilterBindingGetPortDev")
}
-   result := C.virNWFilterBindingGetPortDevWrapper(f.ptr)
+   var err C.virError
+   result := C.virNWFilterBindingGetPortDevWrapper(f.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
name := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -95,9 +99,10 @@ func (f *NWFilterBinding) GetFilterName() (string, error) {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return "", 
GetNotImplementedError("virNWFilterBindingGetFilterName")
}
-   result := C.virNWFilterBindingGetFilterNameWrapper(f.ptr)
+   var err C.virError
+   result := C.virNWFilterBindingGetFilterNameWrapper(f.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
name := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -109,9 +114,10 @@ func (f *NWFilterBinding) GetXMLDesc(flags uint32) 
(string, error) {
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return "", 
GetNotImplementedError("virNWFilterBindingGetXMLDesc")
}
-   result := C.virNWFilterBindingGetXMLDescWrapper(f.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virNWFilterBindingGetXMLDescWrapper(f.ptr, C.uint(flags), 
&err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
xml := C.GoString(result)
C.free(unsafe.Pointer(result))
diff --git a/nwfilter_binding_wrapper.go b/nwfilter_binding_wrapper.go
index fdafaef..b0950b5 100644
--- a/nwfilter_binding_wrapper.go
+++ b/nwfilter_binding_wrapper.go
@@ -30,63 +30,100 @@ package libvirt
 #include 
 #include "nwfilter_binding_wrapper.h"
 
-const char *virNWFilterBindingGetPortDevWrapper(virNWFilterBindingPtr binding)
+
+int
+virNWFilterBindingDeleteWrapper(virNWFilterBindingPtr binding,
+virErrorPtr err)
 {
 #if LIBVIR_VERSION_NUMBER < 4005000
 assert(0); // Caller should have checked version
 #else
-return virNWFilterBindingGetPortDev(binding);
+int ret = virNWFilterBindingDelete(binding);
+if (ret < 0) {
+virCopyLastError(err);
+}
+return ret;
 #endif
 }
 
 
-const char *virNWFilterBindingGetFilterNameWrapper(virNWFilterBindingPtr 
binding)
+int
+virNWFilterBindingFreeWrapper(virNWFilterBindingPtr binding,
+  virErrorPtr err)
 {
 #if LIBVIR_VERSION_NUMBER < 4005000
 assert(0); // Caller should have checked version
 #else
-return virNWFilterBindingGetFilterName(binding);
+int ret = virNWFilterBindingFree(binding);
+if (ret < 0) {
+virCopyLa

[libvirt] [go PATCH 07/37] network: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 network.go  |  1 +
 network_compat.h| 10 ---
 network_events_wrapper.go   | 10 +++
 network_events_wrapper.h|  8 ++
 network_compat.go => network_wrapper.go | 11 +---
 network_wrapper.h   | 37 +
 6 files changed, 57 insertions(+), 20 deletions(-)
 rename network_compat.go => network_wrapper.go (86%)
 create mode 100644 network_wrapper.h

diff --git a/network.go b/network.go
index fe54bf1..26bf24e 100644
--- a/network.go
+++ b/network.go
@@ -32,6 +32,7 @@ package libvirt
 #include 
 #include 
 #include "network_compat.h"
+#include "network_wrapper.h"
 */
 import "C"
 
diff --git a/network_compat.h b/network_compat.h
index 97c8ad5..08f0778 100644
--- a/network_compat.h
+++ b/network_compat.h
@@ -56,9 +56,6 @@ typedef void 
(*virConnectNetworkEventGenericCallback)(virConnectPtr conn,
   void *opaque);
 #endif
 
-int virConnectNetworkEventDeregisterAnyWrapper(virConnectPtr conn,
- int callbackID);
-
 
 /* 1.2.5 */
 
@@ -86,11 +83,4 @@ struct _virNetworkDHCPLease {
 };
 #endif
 
-void virNetworkDHCPLeaseFreeWrapper(virNetworkDHCPLeasePtr lease);
-
-int virNetworkGetDHCPLeasesWrapper(virNetworkPtr network,
- const char *mac,
- virNetworkDHCPLeasePtr **leases,
- unsigned int flags);
-
 #endif /* LIBVIRT_GO_NETWORK_COMPAT_H__ */
diff --git a/network_events_wrapper.go b/network_events_wrapper.go
index ea892cf..27bf555 100644
--- a/network_events_wrapper.go
+++ b/network_events_wrapper.go
@@ -54,5 +54,15 @@ int virConnectNetworkEventRegisterAnyWrapper(virConnectPtr 
c,  virNetworkPtr d,
 #endif
 }
 
+int virConnectNetworkEventDeregisterAnyWrapper(virConnectPtr conn,
+ int callbackID)
+{
+#if LIBVIR_VERSION_NUMBER < 1002001
+assert(0); // Caller should have checked version
+#else
+return virConnectNetworkEventDeregisterAny(conn, callbackID);
+#endif
+}
+
 */
 import "C"
diff --git a/network_events_wrapper.h b/network_events_wrapper.h
index 1b73369..8388467 100644
--- a/network_events_wrapper.h
+++ b/network_events_wrapper.h
@@ -34,5 +34,13 @@ int virConnectNetworkEventRegisterAnyWrapper(virConnectPtr 
c,  virNetworkPtr d,
  int eventID, 
virConnectNetworkEventGenericCallback cb,
  long goCallbackId);
 
+int virConnectNetworkEventRegisterAnyWrapper(virConnectPtr c,
+ virNetworkPtr d,
+ int eventID,
+ 
virConnectNetworkEventGenericCallback cb,
+ long goCallbackId);
+int virConnectNetworkEventDeregisterAnyWrapper(virConnectPtr conn,
+ int callbackID);
+
 
 #endif /* LIBVIRT_GO_NETWORK_EVENTS_WRAPPER_H__ */
diff --git a/network_compat.go b/network_wrapper.go
similarity index 86%
rename from network_compat.go
rename to network_wrapper.go
index 56e9749..bd58754 100644
--- a/network_compat.go
+++ b/network_wrapper.go
@@ -31,16 +31,7 @@ package libvirt
 #include 
 #include 
 #include "network_compat.h"
-
-int virConnectNetworkEventDeregisterAnyWrapper(virConnectPtr conn,
- int callbackID)
-{
-#if LIBVIR_VERSION_NUMBER < 1002001
-assert(0); // Caller should have checked version
-#else
-return virConnectNetworkEventDeregisterAny(conn, callbackID);
-#endif
-}
+#include "network_wrapper.h"
 
 void virNetworkDHCPLeaseFreeWrapper(virNetworkDHCPLeasePtr lease)
 {
diff --git a/network_wrapper.h b/network_wrapper.h
new file mode 100644
index 000..762fe3b
--- /dev/null
+++ b/network_wrapper.h
@@ -0,0 +1,37 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILI

[libvirt] [go PATCH 18/37] Add XXX_wrapper.{h, go} for every remaining file

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 domain_snapshot_wrapper.go | 34 ++
 domain_snapshot_wrapper.h  | 32 
 interface_wrapper.go   | 34 ++
 interface_wrapper.h| 32 
 node_device_wrapper.go | 34 ++
 node_device_wrapper.h  | 33 +
 nwfilter_wrapper.go| 34 ++
 nwfilter_wrapper.h | 32 
 secret_wrapper.go  | 34 ++
 secret_wrapper.h   | 33 +
 storage_pool_wrapper.go| 34 ++
 storage_pool_wrapper.h | 33 +
 12 files changed, 399 insertions(+)
 create mode 100644 domain_snapshot_wrapper.go
 create mode 100644 domain_snapshot_wrapper.h
 create mode 100644 interface_wrapper.go
 create mode 100644 interface_wrapper.h
 create mode 100644 node_device_wrapper.go
 create mode 100644 node_device_wrapper.h
 create mode 100644 nwfilter_wrapper.go
 create mode 100644 nwfilter_wrapper.h
 create mode 100644 secret_wrapper.go
 create mode 100644 secret_wrapper.h
 create mode 100644 storage_pool_wrapper.go
 create mode 100644 storage_pool_wrapper.h

diff --git a/domain_snapshot_wrapper.go b/domain_snapshot_wrapper.go
new file mode 100644
index 000..75e3ea5
--- /dev/null
+++ b/domain_snapshot_wrapper.go
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ */
+
+package libvirt
+
+/*
+#cgo pkg-config: libvirt
+#include 
+#include "domain_snapshot_wrapper.h"
+
+*/
+import "C"
diff --git a/domain_snapshot_wrapper.h b/domain_snapshot_wrapper.h
new file mode 100644
index 000..45464ce
--- /dev/null
+++ b/domain_snapshot_wrapper.h
@@ -0,0 +1,32 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ */
+
+#ifndef LIBVIRT_GO_DOMAIN_SNAPSHOT_WRAPPER_H__
+#define LIBVIRT_GO_DOMAIN_SNAPSHOT_WRAPPER_H__
+
+#include 
+#include 
+
+#endif /* LIBVIRT_GO_DOMAIN_SNAPSHOT_WRAPPER_H__ */
diff --git a/interface_wrapper.go b/interface_wrapper.go
new file mode 100644
index 000..c9618fb
--- /dev/null
+++ b/interface_wrapper.go
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to pe

[libvirt] [go PATCH 22/37] stream: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each stream C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 stream.go |  80 +---
 stream_wrapper.go | 231 +++---
 stream_wrapper.h  |  84 +
 3 files changed, 312 insertions(+), 83 deletions(-)

diff --git a/stream.go b/stream.go
index c5c1ef7..515ae08 100644
--- a/stream.go
+++ b/stream.go
@@ -64,9 +64,10 @@ type Stream struct {
 
 // See also https://libvirt.org/html/libvirt-libvirt-stream.html#virStreamAbort
 func (v *Stream) Abort() error {
-   result := C.virStreamAbort(v.ptr)
+   var err C.virError
+   result := C.virStreamAbortWrapper(v.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
 
return nil
@@ -74,9 +75,10 @@ func (v *Stream) Abort() error {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-stream.html#virStreamFinish
 func (v *Stream) Finish() error {
-   result := C.virStreamFinish(v.ptr)
+   var err C.virError
+   result := C.virStreamFinishWrapper(v.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
 
return nil
@@ -84,27 +86,30 @@ func (v *Stream) Finish() error {
 
 // See also https://libvirt.org/html/libvirt-libvirt-stream.html#virStreamFree
 func (v *Stream) Free() error {
-   ret := C.virStreamFree(v.ptr)
+   var err C.virError
+   ret := C.virStreamFreeWrapper(v.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also https://libvirt.org/html/libvirt-libvirt-stream.html#virStreamRef
 func (c *Stream) Ref() error {
-   ret := C.virStreamRef(c.ptr)
+   var err C.virError
+   ret := C.virStreamRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also https://libvirt.org/html/libvirt-libvirt-stream.html#virStreamRecv
 func (v *Stream) Recv(p []byte) (int, error) {
-   n := C.virStreamRecv(v.ptr, (*C.char)(unsafe.Pointer(&p[0])), 
C.size_t(len(p)))
+   var err C.virError
+   n := C.virStreamRecvWrapper(v.ptr, (*C.char)(unsafe.Pointer(&p[0])), 
C.size_t(len(p)), &err)
if n < 0 {
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
if n == 0 {
return 0, io.EOF
@@ -119,9 +124,10 @@ func (v *Stream) RecvFlags(p []byte, flags 
StreamRecvFlagsValues) (int, error) {
return 0, GetNotImplementedError("virStreamRecvFlags")
}
 
-   n := C.virStreamRecvFlagsWrapper(v.ptr, 
(*C.char)(unsafe.Pointer(&p[0])), C.size_t(len(p)), C.uint(flags))
+   var err C.virError
+   n := C.virStreamRecvFlagsWrapper(v.ptr, 
(*C.char)(unsafe.Pointer(&p[0])), C.size_t(len(p)), C.uint(flags), &err)
if n < 0 {
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
if n == 0 {
return 0, io.EOF
@@ -137,9 +143,10 @@ func (v *Stream) RecvHole(flags uint) (int64, error) {
}
 
var len C.longlong
-   ret := C.virStreamRecvHoleWrapper(v.ptr, &len, C.uint(flags))
+   var err C.virError
+   ret := C.virStreamRecvHoleWrapper(v.ptr, &len, C.uint(flags), &err)
if ret < 0 {
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
 
return int64(len), nil
@@ -147,9 +154,10 @@ func (v *Stream) RecvHole(flags uint) (int64, error) {
 
 // See also https://libvirt.org/html/libvirt-libvirt-stream.html#virStreamSend
 func (v *Stream) Send(p []byte) (int, error) {
-   n := C.virStreamSend(v.ptr, (*C.char)(unsafe.Pointer(&p[0])), 
C.size_t(len(p)))
+   var err C.virError
+   n := C.virStreamSendWrapper(v.ptr, (*C.char)(unsafe.Pointer(&p[0])), 
C.size_t(len(p)), &err)
if n < 0 {
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
if n == 0 {
return 0, io.EOF
@@ -164,9 +172,10 @@ func (v *Stream) SendHole(len int64, flags uint32) error {
return GetNotImplementedError("virStreamSendHole")
}
 
-   ret := C.virStreamSendHoleWrapper(v.ptr, C.longlong(len), C.uint(flags))
+   var err C.virError
+   ret := C.virStreamSendHoleWrapper(v.ptr, C.longlong(len), 
C.uint(flags), &err)
if ret < 0 {
-   return GetLastError()
+   return makeError(&err)
}
 
return nil
@@ -220,10 +229,11 @@ func (v *Stream) RecvAll(handler StreamSinkFunc) error {
 
callbackID := registerCallbackId(handler)
 
-   ret := C.virStreamRecvAllWra

[libvirt] [go PATCH 20/37] storage vol: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each storage volume C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 storage_volume.go |  79 --
 storage_volume_wrapper.go | 211 +-
 storage_volume_wrapper.h  |  79 +-
 3 files changed, 332 insertions(+), 37 deletions(-)

diff --git a/storage_volume.go b/storage_volume.go
index d913c59..711c3c1 100644
--- a/storage_volume.go
+++ b/storage_volume.go
@@ -123,27 +123,30 @@ type StorageVolInfo struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolDelete
 func (v *StorageVol) Delete(flags StorageVolDeleteFlags) error {
-   result := C.virStorageVolDelete(v.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virStorageVolDeleteWrapper(v.ptr, C.uint(flags), &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolFree
 func (v *StorageVol) Free() error {
-   ret := C.virStorageVolFree(v.ptr)
+   var err C.virError
+   ret := C.virStorageVolFreeWrapper(v.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolRef
 func (c *StorageVol) Ref() error {
-   ret := C.virStorageVolRef(c.ptr)
+   var err C.virError
+   ret := C.virStorageVolRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -151,9 +154,10 @@ func (c *StorageVol) Ref() error {
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetInfo
 func (v *StorageVol) GetInfo() (*StorageVolInfo, error) {
var cinfo C.virStorageVolInfo
-   result := C.virStorageVolGetInfo(v.ptr, &cinfo)
+   var err C.virError
+   result := C.virStorageVolGetInfoWrapper(v.ptr, &cinfo, &err)
if result == -1 {
-   return nil, GetLastError()
+   return nil, makeError(&err)
}
return &StorageVolInfo{
Type:   StorageVolType(cinfo._type),
@@ -169,9 +173,10 @@ func (v *StorageVol) GetInfoFlags(flags 
StorageVolInfoFlags) (*StorageVolInfo, e
}
 
var cinfo C.virStorageVolInfo
-   result := C.virStorageVolGetInfoFlagsWrapper(v.ptr, &cinfo, 
C.uint(flags))
+   var err C.virError
+   result := C.virStorageVolGetInfoFlagsWrapper(v.ptr, &cinfo, 
C.uint(flags), &err)
if result == -1 {
-   return nil, GetLastError()
+   return nil, makeError(&err)
}
return &StorageVolInfo{
Type:   StorageVolType(cinfo._type),
@@ -182,27 +187,30 @@ func (v *StorageVol) GetInfoFlags(flags 
StorageVolInfoFlags) (*StorageVolInfo, e
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetKey
 func (v *StorageVol) GetKey() (string, error) {
-   key := C.virStorageVolGetKey(v.ptr)
+   var err C.virError
+   key := C.virStorageVolGetKeyWrapper(v.ptr, &err)
if key == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(key), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetName
 func (v *StorageVol) GetName() (string, error) {
-   name := C.virStorageVolGetName(v.ptr)
+   var err C.virError
+   name := C.virStorageVolGetNameWrapper(v.ptr, &err)
if name == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(name), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetPath
 func (v *StorageVol) GetPath() (string, error) {
-   result := C.virStorageVolGetPath(v.ptr)
+   var err C.virError
+   result := C.virStorageVolGetPathWrapper(v.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
path := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -211,9 +219,10 @@ func (v *StorageVol) GetPath() (string, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetXMLDesc
 func (v *StorageVol) GetXMLDesc(flags uint32) (string, error) {
-   result := C.virStorageVolGetXMLDesc(v.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virStorageVolGetXMLDescWrapper(v.ptr, C.uint(flags), &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(

[libvirt] [go PATCH 15/37] lxc: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 lxc.go  | 2 +-
 lxc_compat.go => lxc_wrapper.go | 2 +-
 lxc_compat.h => lxc_wrapper.h   | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename lxc_compat.go => lxc_wrapper.go (98%)
 rename lxc_compat.h => lxc_wrapper.h (100%)

diff --git a/lxc.go b/lxc.go
index d121231..07ed7ff 100644
--- a/lxc.go
+++ b/lxc.go
@@ -38,7 +38,7 @@ package libvirt
 #include 
 #include 
 #include 
-#include "lxc_compat.h"
+#include "lxc_wrapper.h"
 */
 import "C"
 
diff --git a/lxc_compat.go b/lxc_wrapper.go
similarity index 98%
rename from lxc_compat.go
rename to lxc_wrapper.go
index cc4420c..4a49348 100644
--- a/lxc_compat.go
+++ b/lxc_wrapper.go
@@ -34,7 +34,7 @@ package libvirt
 #include 
 #include 
 #include 
-#include "lxc_compat.h"
+#include "lxc_wrapper.h"
 
 int virDomainLxcEnterCGroupWrapper(virDomainPtr domain,
  unsigned int flags)
diff --git a/lxc_compat.h b/lxc_wrapper.h
similarity index 100%
rename from lxc_compat.h
rename to lxc_wrapper.h
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 23/37] secret: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each secret C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 secret.go |  50 +---
 secret_wrapper.go | 141 ++
 secret_wrapper.h  |  53 +
 3 files changed, 224 insertions(+), 20 deletions(-)

diff --git a/secret.go b/secret.go
index 9bd8b36..c4ef44b 100644
--- a/secret.go
+++ b/secret.go
@@ -67,27 +67,30 @@ type Secret struct {
 
 // See also https://libvirt.org/html/libvirt-libvirt-secret.html#virSecretFree
 func (s *Secret) Free() error {
-   ret := C.virSecretFree(s.ptr)
+   var err C.virError
+   ret := C.virSecretFreeWrapper(s.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also https://libvirt.org/html/libvirt-libvirt-secret.html#virSecretRef
 func (c *Secret) Ref() error {
-   ret := C.virSecretRef(c.ptr)
+   var err C.virError
+   ret := C.virSecretRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-secret.html#virSecretUndefine
 func (s *Secret) Undefine() error {
-   result := C.virSecretUndefine(s.ptr)
+   var err C.virError
+   result := C.virSecretUndefineWrapper(s.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
@@ -96,9 +99,10 @@ func (s *Secret) Undefine() error {
 func (s *Secret) GetUUID() ([]byte, error) {
var cUuid [C.VIR_UUID_BUFLEN](byte)
cuidPtr := unsafe.Pointer(&cUuid)
-   result := C.virSecretGetUUID(s.ptr, (*C.uchar)(cuidPtr))
+   var err C.virError
+   result := C.virSecretGetUUIDWrapper(s.ptr, (*C.uchar)(cuidPtr), &err)
if result != 0 {
-   return []byte{}, GetLastError()
+   return []byte{}, makeError(&err)
}
return C.GoBytes(cuidPtr, C.VIR_UUID_BUFLEN), nil
 }
@@ -107,36 +111,40 @@ func (s *Secret) GetUUID() ([]byte, error) {
 func (s *Secret) GetUUIDString() (string, error) {
var cUuid [C.VIR_UUID_STRING_BUFLEN](C.char)
cuidPtr := unsafe.Pointer(&cUuid)
-   result := C.virSecretGetUUIDString(s.ptr, (*C.char)(cuidPtr))
+   var err C.virError
+   result := C.virSecretGetUUIDStringWrapper(s.ptr, (*C.char)(cuidPtr), 
&err)
if result != 0 {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString((*C.char)(cuidPtr)), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-secret.html#virSecretGetUsageID
 func (s *Secret) GetUsageID() (string, error) {
-   result := C.virSecretGetUsageID(s.ptr)
+   var err C.virError
+   result := C.virSecretGetUsageIDWrapper(s.ptr, &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(result), nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-secret.html#virSecretGetUsageType
 func (s *Secret) GetUsageType() (SecretUsageType, error) {
-   result := SecretUsageType(C.virSecretGetUsageType(s.ptr))
+   var err C.virError
+   result := SecretUsageType(C.virSecretGetUsageTypeWrapper(s.ptr, &err))
if result == -1 {
-   return 0, GetLastError()
+   return 0, makeError(&err)
}
return result, nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-secret.html#virSecretGetXMLDesc
 func (s *Secret) GetXMLDesc(flags uint32) (string, error) {
-   result := C.virSecretGetXMLDesc(s.ptr, C.uint(flags))
+   var err C.virError
+   result := C.virSecretGetXMLDescWrapper(s.ptr, C.uint(flags), &err)
if result == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
xml := C.GoString(result)
C.free(unsafe.Pointer(result))
@@ -147,9 +155,10 @@ func (s *Secret) GetXMLDesc(flags uint32) (string, error) {
 func (s *Secret) GetValue(flags uint32) ([]byte, error) {
var cvalue_size C.size_t
 
-   cvalue := C.virSecretGetValue(s.ptr, &cvalue_size, C.uint(flags))
+   var err C.virError
+   cvalue := C.virSecretGetValueWrapper(s.ptr, &cvalue_size, 
C.uint(flags), &err)
if cvalue == nil {
-   return nil, GetLastError()
+   return nil, makeError(&err)
}
defer C.free(unsafe.Pointer(cvalue))
ret := C.GoBytes(unsafe.Pointer(cvalue), C.int(cvalue_size))
@@ -164,10 +173,11 @@ func (s *Secret) SetValue(value []byte, flags uint32) 
error {
cvalue[i] = C.uchar(value[i])

[libvirt] [go PATCH 13/37] storage pool: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 storage_pool_compat.go | 46 --
 storage_pool_compat.h  |  4 ---
 storage_pool_events_wrapper.go | 10 
 storage_pool_events_wrapper.h  |  2 ++
 4 files changed, 12 insertions(+), 50 deletions(-)
 delete mode 100644 storage_pool_compat.go

diff --git a/storage_pool_compat.go b/storage_pool_compat.go
deleted file mode 100644
index 7ad66b3..000
--- a/storage_pool_compat.go
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * This file is part of the libvirt-go project
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * Copyright (c) 2013 Alex Zorin
- * Copyright (C) 2016 Red Hat, Inc.
- *
- */
-
-package libvirt
-
-/*
-#cgo pkg-config: libvirt
-#include 
-#include 
-#include "storage_pool_compat.h"
-
-int virConnectStoragePoolEventDeregisterAnyWrapper(virConnectPtr conn,
- int callbackID)
-{
-#if LIBVIR_VERSION_NUMBER < 200
-assert(0); // Caller shouuld have checked version
-#else
-return virConnectStoragePoolEventDeregisterAny(conn, callbackID);
-#endif
-}
-
-*/
-import "C"
diff --git a/storage_pool_compat.h b/storage_pool_compat.h
index cad74fd..4df1a0d 100644
--- a/storage_pool_compat.h
+++ b/storage_pool_compat.h
@@ -78,10 +78,6 @@ typedef void 
(*virConnectStoragePoolEventGenericCallback)(virConnectPtr conn,
   void *opaque);
 #endif
 
-int virConnectStoragePoolEventDeregisterAnyWrapper(virConnectPtr conn,
- int callbackID);
-
-
 /* 3.8.0 */
 
 #ifndef VIR_STORAGE_POOL_EVENT_CREATED
diff --git a/storage_pool_events_wrapper.go b/storage_pool_events_wrapper.go
index 9670440..d03f6c5 100644
--- a/storage_pool_events_wrapper.go
+++ b/storage_pool_events_wrapper.go
@@ -61,5 +61,15 @@ int 
virConnectStoragePoolEventRegisterAnyWrapper(virConnectPtr c,  virStoragePoo
 #endif
 }
 
+int virConnectStoragePoolEventDeregisterAnyWrapper(virConnectPtr conn,
+ int callbackID)
+{
+#if LIBVIR_VERSION_NUMBER < 200
+assert(0); // Caller shouuld have checked version
+#else
+return virConnectStoragePoolEventDeregisterAny(conn, callbackID);
+#endif
+}
+
 */
 import "C"
diff --git a/storage_pool_events_wrapper.h b/storage_pool_events_wrapper.h
index c82abb2..2420dee 100644
--- a/storage_pool_events_wrapper.h
+++ b/storage_pool_events_wrapper.h
@@ -36,5 +36,7 @@ int 
virConnectStoragePoolEventRegisterAnyWrapper(virConnectPtr c,  virStoragePoo
  int eventID, 
virConnectStoragePoolEventGenericCallback cb,
  long goCallbackId);
 
+int virConnectStoragePoolEventDeregisterAnyWrapper(virConnectPtr conn,
+ int callbackID);
 
 #endif /* LIBVIRT_GO_STORAGE_POOL_EVENTS_WRAPPER_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 19/37] Standardize formatting in all wrapper headers

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 connect.go|   2 +-
 connect_wrapper.go|   2 +-
 connect_wrapper.h | 169 +++-
 domain_events_wrapper.h   | 258 +++---
 domain_snapshot.go|   3 +-
 domain_wrapper.h  | 290 --
 events_wrapper.h  |  39 -
 interface.go  |   3 +-
 lxc_wrapper.h |   5 +-
 network_events_wrapper.h  |  32 ++--
 network_wrapper.h |  12 +-
 node_device.go|   4 +-
 node_device_events_wrapper.h  |  29 ++--
 nwfilter.go   |   3 +-
 nwfilter_binding_wrapper.h|  25 ++-
 qemu_wrapper.h|  26 ++-
 secret.go |   4 +-
 secret_events_wrapper.h   |  26 ++-
 storage_pool.go   |   4 +-
 storage_pool_events_wrapper.h |  29 ++--
 storage_volume_wrapper.h  |   7 +-
 stream_wrapper.h  |  57 ---
 22 files changed, 638 insertions(+), 391 deletions(-)

diff --git a/connect.go b/connect.go
index 639225b..5ea8c48 100644
--- a/connect.go
+++ b/connect.go
@@ -395,7 +395,7 @@ func NewConnectWithAuth(uri string, auth *ConnectAuth, 
flags ConnectFlags) (*Con
 
callbackID := registerCallbackId(auth.Callback)
 
-   ptr := C.virConnectOpenAuthWrap(cUri, &ccredtype[0], 
C.uint(len(auth.CredType)), C.int(callbackID), C.uint(flags))
+   ptr := C.virConnectOpenAuthWrapper(cUri, &ccredtype[0], 
C.uint(len(auth.CredType)), C.int(callbackID), C.uint(flags))
freeCallbackId(callbackID)
if ptr == nil {
return nil, GetLastError()
diff --git a/connect_wrapper.go b/connect_wrapper.go
index 82f0ba7..d714fb9 100644
--- a/connect_wrapper.go
+++ b/connect_wrapper.go
@@ -54,7 +54,7 @@ int connectAuthCallbackHelper(virConnectCredentialPtr cred, 
unsigned int ncred,
 return connectAuthCallback(cred, ncred, *callbackID);
 }
 
-virConnectPtr virConnectOpenAuthWrap(const char *name, int *credtype, uint 
ncredtype, int callbackID, unsigned int flags)
+virConnectPtr virConnectOpenAuthWrapper(const char *name, int *credtype, uint 
ncredtype, int callbackID, unsigned int flags)
 {
 virConnectAuth auth = {
.credtype = credtype,
diff --git a/connect_wrapper.h b/connect_wrapper.h
index 0fdde02..726d29c 100644
--- a/connect_wrapper.h
+++ b/connect_wrapper.h
@@ -31,84 +31,111 @@
 #include 
 #include "connect_compat.h"
 
-void closeCallbackHelper(virConnectPtr conn, int reason, void *opaque);
-int virConnectRegisterCloseCallbackHelper(virConnectPtr c, virConnectCloseFunc 
cb, long goCallbackId);
-
-virConnectPtr virConnectOpenAuthWrap(const char *name, int *credtype, uint 
ncredtype, int callbackID, unsigned int flags);
-
-int virNodeGetFreePagesWrapper(virConnectPtr conn,
-   unsigned int npages,
-   unsigned int *pages,
-   int startcell,
-   unsigned int cellcount,
-   unsigned long long *counts,
-   unsigned int flags);
-
-char * virConnectGetDomainCapabilitiesWrapper(virConnectPtr conn,
-  const char *emulatorbin,
-  const char *arch,
-  const char *machine,
-  const char *virttype,
-  unsigned int flags);
-
-int virConnectGetAllDomainStatsWrapper(virConnectPtr conn,
-   unsigned int stats,
-   virDomainStatsRecordPtr **retStats,
+void
+closeCallbackHelper(virConnectPtr conn,
+int reason,
+void *opaque);
+
+int
+virConnectRegisterCloseCallbackHelper(virConnectPtr c,
+  virConnectCloseFunc cb,
+  long goCallbackId);
+
+virConnectPtr
+virConnectOpenAuthWrapper(const char *name,
+  int *credtype,
+  uint ncredtype,
+  int callbackID,
+  unsigned int flags);
+
+int
+virNodeGetFreePagesWrapper(virConnectPtr conn,
+   unsigned int npages,
+   unsigned int *pages,
+   int startcell,
+   unsigned int cellcount,
+   unsigned long long *counts,
+   unsigned int flags);
+
+char *
+virConnectGetDomainCapabilitiesWrapper(virConnectPtr conn,
+   const char *emulatorbin,
+   const char *arch,
+   const char *machine,
+   const char *virttype,
  

[libvirt] [go PATCH 14/37] qemu: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 qemu_compat.go  | 48 
 qemu_compat.h   |  3 ---
 qemu_wrapper.go | 10 ++
 qemu_wrapper.h  |  3 +++
 4 files changed, 13 insertions(+), 51 deletions(-)
 delete mode 100644 qemu_compat.go

diff --git a/qemu_compat.go b/qemu_compat.go
deleted file mode 100644
index 6cd28b0..000
--- a/qemu_compat.go
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * This file is part of the libvirt-go project
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * Copyright (c) 2013 Alex Zorin
- * Copyright (C) 2016 Red Hat, Inc.
- *
- */
-
-package libvirt
-
-/*
-#cgo pkg-config: libvirt
-#include 
-#include 
-#include 
-#include "qemu_compat.h"
-
-
-int virConnectDomainQemuMonitorEventDeregisterWrapper(virConnectPtr conn,
-int callbackID)
-{
-#if LIBVIR_VERSION_NUMBER < 1002003
-assert(0); // Caller should have checked version
-#else
-return virConnectDomainQemuMonitorEventDeregister(conn, callbackID);
-#endif
-}
-
-*/
-import "C"
diff --git a/qemu_compat.h b/qemu_compat.h
index ad556cc..7756d7f 100644
--- a/qemu_compat.h
+++ b/qemu_compat.h
@@ -39,9 +39,6 @@ typedef void 
(*virConnectDomainQemuMonitorEventCallback)(virConnectPtr conn,
  void *opaque);
 #endif
 
-int virConnectDomainQemuMonitorEventDeregisterWrapper(virConnectPtr conn,
-int callbackID);
-
 #ifndef VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX
 #define VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX (1 << 0)
 #endif
diff --git a/qemu_wrapper.go b/qemu_wrapper.go
index bbd95cc..ff837a0 100644
--- a/qemu_wrapper.go
+++ b/qemu_wrapper.go
@@ -60,5 +60,15 @@ int 
virConnectDomainQemuMonitorEventRegisterWrapper(virConnectPtr c,  virDomainP
 #endif
 }
 
+int virConnectDomainQemuMonitorEventDeregisterWrapper(virConnectPtr conn,
+int callbackID)
+{
+#if LIBVIR_VERSION_NUMBER < 1002003
+assert(0); // Caller should have checked version
+#else
+return virConnectDomainQemuMonitorEventDeregister(conn, callbackID);
+#endif
+}
+
 */
 import "C"
diff --git a/qemu_wrapper.h b/qemu_wrapper.h
index 9284256..3f4dd6a 100644
--- a/qemu_wrapper.h
+++ b/qemu_wrapper.h
@@ -35,5 +35,8 @@ int 
virConnectDomainQemuMonitorEventRegisterWrapper(virConnectPtr c,  virDomainP
 const char *event, 
virConnectDomainQemuMonitorEventCallback cb,
 long goCallbackId, unsigned 
int flags);
 
+int virConnectDomainQemuMonitorEventDeregisterWrapper(virConnectPtr conn,
+int callbackID);
+
 
 #endif /* LIBVIRT_GO_DOMAIN_EVENTS_WRAPPER_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 35/37] error: remove GetLastError() function

2018-07-16 Thread Daniel P . Berrangé
The virGetLastError() function fetches the last reported error from a
thread local variable. Goroutines may be arbitrarily switched between OS
threads between the libvirt API call and the virGetLastError()
call. Thus this API is impossible to use safely and must be removed. All
the Go APIs return an error object directly so nothing should need the
GetLastError() binding anyway.

Signed-off-by: Daniel P. Berrangé 
---
 api_test.go |  1 +
 error.go| 20 
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/api_test.go b/api_test.go
index a16a1eb..564bdd4 100644
--- a/api_test.go
+++ b/api_test.go
@@ -68,6 +68,7 @@ var (
/* Only needed at C level */
"virCopyLastError",
"virFreeError",
+   "virGetLastError",
"virGetLastErrorMessage",
"virGetLastErrorCode",
"virGetLastErrorDomain",
diff --git a/error.go b/error.go
index b8e2900..37fefc1 100644
--- a/error.go
+++ b/error.go
@@ -594,26 +594,6 @@ func makeError(err *C.virError) Error {
return ret
 }
 
-func GetLastError() Error {
-   err := C.virGetLastError()
-   if err == nil {
-   return Error{
-   Code:ERR_OK,
-   Domain:  FROM_NONE,
-   Message: "Missing error",
-   Level:   ERR_NONE,
-   }
-   }
-   virErr := Error{
-   Code:ErrorNumber(err.code),
-   Domain:  ErrorDomain(err.domain),
-   Message: C.GoString(err.message),
-   Level:   ErrorLevel(err.level),
-   }
-   C.virResetError(err)
-   return virErr
-}
-
 func GetNotImplementedError(apiname string) Error {
return Error{
Code:ERR_NO_SUPPORT,
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 17/37] make the XXX_wrapper.h header files self-contained

2018-07-16 Thread Daniel P . Berrangé
Pull in the main libvirt headers and compat headers from XXX_wrapper.h
so that they are self-contained

Signed-off-by: Daniel P. Berrangé 
---
 callbacks_wrapper.go   | 2 --
 connect.go | 3 ---
 connect_wrapper.go | 2 --
 connect_wrapper.h  | 5 +
 domain.go  | 3 ---
 domain_events.go   | 2 --
 domain_events_wrapper.go   | 2 --
 domain_events_wrapper.h| 4 
 domain_wrapper.go  | 1 -
 domain_wrapper.h   | 3 +++
 events.go  | 1 -
 events_wrapper.go  | 2 --
 events_wrapper.h   | 3 +++
 lxc.go | 3 ---
 lxc_wrapper.go | 2 --
 lxc_wrapper.h  | 4 +++-
 network.go | 3 ---
 network_events.go  | 2 --
 network_events_wrapper.go  | 5 +
 network_events_wrapper.h   | 4 
 network_wrapper.go | 2 --
 network_wrapper.h  | 4 
 node_device_events.go  | 2 --
 node_device_events_wrapper.go  | 5 +
 node_device_events_wrapper.h   | 4 
 nwfilter_binding.go| 3 ---
 nwfilter_binding_wrapper.go| 2 --
 nwfilter_binding_wrapper.h | 4 
 qemu.go| 4 
 qemu_wrapper.go| 8 ++--
 qemu_wrapper.h | 5 +
 secret_events.go   | 2 --
 secret_events_wrapper.go   | 5 +
 secret_events_wrapper.h| 4 
 storage_pool_events.go | 2 --
 storage_pool_events_wrapper.go | 3 ---
 storage_pool_events_wrapper.h  | 4 
 storage_volume.go  | 3 ---
 storage_volume_wrapper.go  | 2 --
 storage_volume_wrapper.h   | 4 
 stream.go  | 3 ---
 stream_wrapper.go  | 2 --
 stream_wrapper.h   | 4 
 43 files changed, 60 insertions(+), 77 deletions(-)

diff --git a/callbacks_wrapper.go b/callbacks_wrapper.go
index 64caf69..9716bb1 100644
--- a/callbacks_wrapper.go
+++ b/callbacks_wrapper.go
@@ -28,8 +28,6 @@ package libvirt
 
 /*
 #cgo pkg-config: libvirt
-#include 
-#include 
 #include "callbacks_wrapper.h"
 
 extern void freeCallbackId(long);
diff --git a/connect.go b/connect.go
index 98ab7ab..639225b 100644
--- a/connect.go
+++ b/connect.go
@@ -36,10 +36,7 @@ import (
 
 /*
 #cgo pkg-config: libvirt
-#include 
-#include 
 #include 
-#include "connect_compat.h"
 #include "connect_wrapper.h"
 */
 import "C"
diff --git a/connect_wrapper.go b/connect_wrapper.go
index cbafc53..82f0ba7 100644
--- a/connect_wrapper.go
+++ b/connect_wrapper.go
@@ -28,8 +28,6 @@ package libvirt
 
 /*
 #cgo pkg-config: libvirt
-#include 
-#include 
 #include 
 #include "connect_wrapper.h"
 #include "callbacks_wrapper.h"
diff --git a/connect_wrapper.h b/connect_wrapper.h
index 7af88f1..0fdde02 100644
--- a/connect_wrapper.h
+++ b/connect_wrapper.h
@@ -26,6 +26,11 @@
 
 #ifndef LIBVIRT_GO_CONNECT_WRAPPER_H__
 #define LIBVIRT_GO_CONNECT_WRAPPER_H__
+
+#include 
+#include 
+#include "connect_compat.h"
+
 void closeCallbackHelper(virConnectPtr conn, int reason, void *opaque);
 int virConnectRegisterCloseCallbackHelper(virConnectPtr c, virConnectCloseFunc 
cb, long goCallbackId);
 
diff --git a/domain.go b/domain.go
index 7253c67..fad2a41 100644
--- a/domain.go
+++ b/domain.go
@@ -28,10 +28,7 @@ package libvirt
 
 /*
 #cgo pkg-config: libvirt
-#include 
-#include 
 #include 
-#include "domain_compat.h"
 #include "domain_wrapper.h"
 */
 import "C"
diff --git a/domain_events.go b/domain_events.go
index 5d21b01..68dc301 100644
--- a/domain_events.go
+++ b/domain_events.go
@@ -33,9 +33,7 @@ import (
 
 /*
 #cgo pkg-config: libvirt
-#include 
 #include "domain_events_wrapper.h"
-#include "domain_compat.h"
 */
 import "C"
 
diff --git a/domain_events_wrapper.go b/domain_events_wrapper.go
index 799945b..07b9418 100644
--- a/domain_events_wrapper.go
+++ b/domain_events_wrapper.go
@@ -28,8 +28,6 @@ package libvirt
 
 /*
 #cgo pkg-config: libvirt
-#include 
-#include 
 #include "domain_events_wrapper.h"
 #include "callbacks_wrapper.h"
 #include 
diff --git a/domain_events_wrapper.h b/domain_events_wrapper.h
index 55cf48e..a9c98c1 100644
--- a/domain_events_wrapper.h
+++ b/domain_events_wrapper.h
@@ -27,6 +27,10 @@
 #ifndef LIBVIRT_GO_DOMAIN_EVENTS_WRAPPER_H__
 #define LIBVIRT_GO_DOMAIN_EVENTS_WRAPPER_H__
 
+#include 
+#include 
+#include "domain_compat.h"
+
 void domainEventLifecycleCallbackHelper(virConnectPtr c, virDomainPtr d,
  int event, int detail, void* data);
 
diff --git a/domain_wrapper.go b/domain_wrapper.go
index 209af78..1596ae5 100644
--- a/domain_wrapper.go
+++ b/domain_wrapper.go
@@ -28,7 +28,6 @@ package libvirt
 
 /*
 #cgo pkg-config: libvirt
-#include 
 #include 
 #include "domain_wrapper.h"
 
diff --git a/domain_wrapper.h b/domain_wrapper.h
index ab2138e..7f857d7 100644
--- a/domain_wrapper.h
+++ b/domain_wrapper.h
@@ -27,6 +27,9 @@
 #ifndef LIBVIRT_G

[libvirt] [go PATCH 27/37] network: fix error reporting thread safety

2018-07-16 Thread Daniel P . Berrangé
Create wrapper functions for each network C API that accepts a
virErrorPtr parameter. This avoids accessing a thread local from a
goroutine which may race with other goroutines doing native API calls in
the same OS thread.

Signed-off-by: Daniel P. Berrangé 
---
 network.go |  80 +---
 network_wrapper.go | 228 +++--
 network_wrapper.h  |  78 +++-
 3 files changed, 347 insertions(+), 39 deletions(-)

diff --git a/network.go b/network.go
index 015fe5e..d9ec9bf 100644
--- a/network.go
+++ b/network.go
@@ -121,45 +121,50 @@ type NetworkDHCPLease struct {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkFree
 func (n *Network) Free() error {
-   ret := C.virNetworkFree(n.ptr)
+   var err C.virError
+   ret := C.virNetworkFreeWrapper(n.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkRef
 func (c *Network) Ref() error {
-   ret := C.virNetworkRef(c.ptr)
+   var err C.virError
+   ret := C.virNetworkRefWrapper(c.ptr, &err)
if ret == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkCreate
 func (n *Network) Create() error {
-   result := C.virNetworkCreate(n.ptr)
+   var err C.virError
+   result := C.virNetworkCreateWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkDestroy
 func (n *Network) Destroy() error {
-   result := C.virNetworkDestroy(n.ptr)
+   var err C.virError
+   result := C.virNetworkDestroyWrapper(n.ptr, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsActive
 func (n *Network) IsActive() (bool, error) {
-   result := C.virNetworkIsActive(n.ptr)
+   var err C.virError
+   result := C.virNetworkIsActiveWrapper(n.ptr, &err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
if result == 1 {
return true, nil
@@ -169,9 +174,10 @@ func (n *Network) IsActive() (bool, error) {
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsPersistent
 func (n *Network) IsPersistent() (bool, error) {
-   result := C.virNetworkIsPersistent(n.ptr)
+   var err C.virError
+   result := C.virNetworkIsPersistentWrapper(n.ptr, &err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
if result == 1 {
return true, nil
@@ -182,9 +188,10 @@ func (n *Network) IsPersistent() (bool, error) {
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetAutostart
 func (n *Network) GetAutostart() (bool, error) {
var out C.int
-   result := C.virNetworkGetAutostart(n.ptr, 
(*C.int)(unsafe.Pointer(&out)))
+   var err C.virError
+   result := C.virNetworkGetAutostartWrapper(n.ptr, 
(*C.int)(unsafe.Pointer(&out)), &err)
if result == -1 {
-   return false, GetLastError()
+   return false, makeError(&err)
}
switch out {
case 1:
@@ -203,18 +210,20 @@ func (n *Network) SetAutostart(autostart bool) error {
default:
cAutostart = 0
}
-   result := C.virNetworkSetAutostart(n.ptr, cAutostart)
+   var err C.virError
+   result := C.virNetworkSetAutostartWrapper(n.ptr, cAutostart, &err)
if result == -1 {
-   return GetLastError()
+   return makeError(&err)
}
return nil
 }
 
 // See also 
https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetName
 func (n *Network) GetName() (string, error) {
-   name := C.virNetworkGetName(n.ptr)
+   var err C.virError
+   name := C.virNetworkGetNameWrapper(n.ptr, &err)
if name == nil {
-   return "", GetLastError()
+   return "", makeError(&err)
}
return C.GoString(name), nil
 }
@@ -223,9 +232,10 @@ func (n *Network) GetName() (string, error) {
 func (n *Network) GetUUID() ([]byte, error) {
var cUuid [C.VIR_UUID_BUFLEN](byte)
cuidPtr := unsafe.Pointer(&cUuid)
-   result := C.virNetworkGetUUID(n.ptr, (*C.uchar)(cuidPtr))
+   var err C.virError
+   result := C.virNetworkGetUUIDWrapper(n.ptr, (*C.uchar)(cuidPtr), &err)
if result != 0 {
-   retu

[libvirt] [go PATCH 10/37] secret: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 secret_compat.go | 47 
 secret_compat.h  |  3 ---
 secret_events_wrapper.go | 11 ++
 secret_events_wrapper.h  |  6 +++--
 4 files changed, 15 insertions(+), 52 deletions(-)
 delete mode 100644 secret_compat.go

diff --git a/secret_compat.go b/secret_compat.go
deleted file mode 100644
index f6b..000
--- a/secret_compat.go
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * This file is part of the libvirt-go project
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * Copyright (c) 2013 Alex Zorin
- * Copyright (C) 2016 Red Hat, Inc.
- *
- */
-
-package libvirt
-
-/*
-#cgo pkg-config: libvirt
-#include 
-#include 
-#include "secret_compat.h"
-
-int virConnectSecretEventDeregisterAnyWrapper(virConnectPtr conn,
- int callbackID)
-{
-#if LIBVIR_VERSION_NUMBER < 300
-assert(0); // Caller should have checked version
-#else
-return virConnectSecretEventDeregisterAny(conn, callbackID);
-#endif
-}
-
-
-*/
-import "C"
diff --git a/secret_compat.h b/secret_compat.h
index d1e1b8a..ba38c68 100644
--- a/secret_compat.h
+++ b/secret_compat.h
@@ -52,9 +52,6 @@ typedef void 
(*virConnectSecretEventGenericCallback)(virConnectPtr conn,
 void *opaque);
 #endif
 
-int virConnectSecretEventDeregisterAnyWrapper(virConnectPtr conn,
-int callbackID);
-
 /* 2.2.1 */
 
 #ifndef VIR_SECRET_USAGE_TYPE_TLS
diff --git a/secret_events_wrapper.go b/secret_events_wrapper.go
index 3366065..6e1ef21 100644
--- a/secret_events_wrapper.go
+++ b/secret_events_wrapper.go
@@ -61,5 +61,16 @@ int virConnectSecretEventRegisterAnyWrapper(virConnectPtr c, 
 virSecretPtr d,
 #endif
 }
 
+int virConnectSecretEventDeregisterAnyWrapper(virConnectPtr conn,
+ int callbackID)
+{
+#if LIBVIR_VERSION_NUMBER < 300
+assert(0); // Caller should have checked version
+#else
+return virConnectSecretEventDeregisterAny(conn, callbackID);
+#endif
+}
+
+
 */
 import "C"
diff --git a/secret_events_wrapper.h b/secret_events_wrapper.h
index 1b5b527..f388542 100644
--- a/secret_events_wrapper.h
+++ b/secret_events_wrapper.h
@@ -28,13 +28,15 @@
 #define LIBVIRT_GO_SECRET_EVENTS_WRAPPER_H__
 
 void secretEventLifecycleCallbackHelper(virConnectPtr c, virSecretPtr d,
- int event, int detail, void* data);
+ int event, int detail, void* data);
 void secretEventGenericCallbackHelper(virConnectPtr c, virSecretPtr d,
-   void* data);
+   void* data);
 
 int virConnectSecretEventRegisterAnyWrapper(virConnectPtr c,  virSecretPtr d,
  int eventID, 
virConnectSecretEventGenericCallback cb,
  long goCallbackId);
 
+int virConnectSecretEventDeregisterAnyWrapper(virConnectPtr conn,
+  int callbackID);
 
 #endif /* LIBVIRT_GO_SECRET_EVENTS_WRAPPER_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 09/37] node device: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 node_device_compat.go | 46 ---
 node_device_compat.h  |  3 ---
 node_device_events_wrapper.go | 10 
 node_device_events_wrapper.h  |  9 ---
 4 files changed, 16 insertions(+), 52 deletions(-)
 delete mode 100644 node_device_compat.go

diff --git a/node_device_compat.go b/node_device_compat.go
deleted file mode 100644
index f2d32d9..000
--- a/node_device_compat.go
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * This file is part of the libvirt-go project
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * Copyright (c) 2013 Alex Zorin
- * Copyright (C) 2016 Red Hat, Inc.
- *
- */
-
-package libvirt
-
-/*
-#cgo pkg-config: libvirt
-#include 
-#include 
-#include "node_device_compat.h"
-
-int virConnectNodeDeviceEventDeregisterAnyWrapper(virConnectPtr conn,
-int callbackID)
-{
-#if LIBVIR_VERSION_NUMBER < 2002000
-assert(0); // Caller should have checked version
-#else
-return virConnectNodeDeviceEventDeregisterAny(conn, callbackID);
-#endif
-}
-
-*/
-import "C"
diff --git a/node_device_compat.h b/node_device_compat.h
index e214bd6..3202039 100644
--- a/node_device_compat.h
+++ b/node_device_compat.h
@@ -51,8 +51,5 @@ typedef void 
(*virConnectNodeDeviceEventGenericCallback)(virConnectPtr conn,
  void *opaque);
 #endif
 
-int virConnectNodeDeviceEventDeregisterAnyWrapper(virConnectPtr conn,
-int callbackID);
-
 
 #endif /* LIBVIRT_GO_NODE_DEVICE_COMPAT_H__ */
diff --git a/node_device_events_wrapper.go b/node_device_events_wrapper.go
index ff7b6e4..20441f4 100644
--- a/node_device_events_wrapper.go
+++ b/node_device_events_wrapper.go
@@ -60,5 +60,15 @@ int 
virConnectNodeDeviceEventRegisterAnyWrapper(virConnectPtr c,  virNodeDeviceP
 #endif
 }
 
+int virConnectNodeDeviceEventDeregisterAnyWrapper(virConnectPtr conn,
+int callbackID)
+{
+#if LIBVIR_VERSION_NUMBER < 2002000
+assert(0); // Caller should have checked version
+#else
+return virConnectNodeDeviceEventDeregisterAny(conn, callbackID);
+#endif
+}
+
 */
 import "C"
diff --git a/node_device_events_wrapper.h b/node_device_events_wrapper.h
index ae76b3e..a9576e7 100644
--- a/node_device_events_wrapper.h
+++ b/node_device_events_wrapper.h
@@ -28,13 +28,16 @@
 #define LIBVIRT_GO_NODE_DEVICE_EVENTS_WRAPPER_H__
 
 void nodeDeviceEventLifecycleCallbackHelper(virConnectPtr c, virNodeDevicePtr 
d,
- int event, int detail, void* data);
+ int event, int detail, void* data);
 
 void nodeDeviceEventGenericCallbackHelper(virConnectPtr c, virNodeDevicePtr d, 
void* data);
 
 int virConnectNodeDeviceEventRegisterAnyWrapper(virConnectPtr c,  
virNodeDevicePtr d,
-int eventID, 
virConnectNodeDeviceEventGenericCallback cb,
-long goCallbackId);
+int eventID, 
virConnectNodeDeviceEventGenericCallback cb,
+long goCallbackId);
+
+int virConnectNodeDeviceEventDeregisterAnyWrapper(virConnectPtr conn,
+int callbackID);
 
 
 #endif /* LIBVIRT_GO_NODE_DEVICE_EVENTS_WRAPPER_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 12/37] storage volume: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 storage_volume.go |  1 +
 storage_volume_compat.h   |  4 ---
 ...ume_compat.go => storage_volume_wrapper.go |  1 +
 storage_volume_wrapper.h  | 34 +++
 4 files changed, 36 insertions(+), 4 deletions(-)
 rename storage_volume_compat.go => storage_volume_wrapper.go (97%)
 create mode 100644 storage_volume_wrapper.h

diff --git a/storage_volume.go b/storage_volume.go
index 56a3026..3526ed4 100644
--- a/storage_volume.go
+++ b/storage_volume.go
@@ -32,6 +32,7 @@ package libvirt
 #include 
 #include 
 #include "storage_volume_compat.h"
+#include "storage_volume_wrapper.h"
 */
 import "C"
 
diff --git a/storage_volume_compat.h b/storage_volume_compat.h
index 69e2dd8..bd3cee9 100644
--- a/storage_volume_compat.h
+++ b/storage_volume_compat.h
@@ -29,10 +29,6 @@
 
 /* 3.0.0 */
 
-int virStorageVolGetInfoFlagsWrapper(virStorageVolPtr vol,
-   virStorageVolInfoPtr info,
-   unsigned int flags);
-
 #ifndef VIR_STORAGE_VOL_USE_ALLOCATION
 #define VIR_STORAGE_VOL_USE_ALLOCATION 0
 #endif
diff --git a/storage_volume_compat.go b/storage_volume_wrapper.go
similarity index 97%
rename from storage_volume_compat.go
rename to storage_volume_wrapper.go
index a1f622e..62c5df8 100644
--- a/storage_volume_compat.go
+++ b/storage_volume_wrapper.go
@@ -31,6 +31,7 @@ package libvirt
 #include 
 #include 
 #include "storage_volume_compat.h"
+#include "storage_volume_wrapper.h"
 
 int virStorageVolGetInfoFlagsWrapper(virStorageVolPtr vol,
virStorageVolInfoPtr info,
diff --git a/storage_volume_wrapper.h b/storage_volume_wrapper.h
new file mode 100644
index 000..a97a59a
--- /dev/null
+++ b/storage_volume_wrapper.h
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (c) 2013 Alex Zorin
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ */
+
+#ifndef LIBVIRT_GO_STORAGE_VOLUME_WRAPPER_H__
+#define LIBVIRT_GO_STORAGE_VOLUME_WRAPPER_H__
+
+int virStorageVolGetInfoFlagsWrapper(virStorageVolPtr vol,
+   virStorageVolInfoPtr info,
+   unsigned int flags);
+
+#endif /* LIBVIRT_GO_STORAGE_VOLUME_WRAPPER_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 08/37] nwfilter binding: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 nwfilter_binding.go   |  1 +
 nwfilter_binding_compat.h | 11 --
 ...g_compat.go => nwfilter_binding_wrapper.go |  1 +
 nwfilter_binding_wrapper.h| 38 +++
 4 files changed, 40 insertions(+), 11 deletions(-)
 rename nwfilter_binding_compat.go => nwfilter_binding_wrapper.go (98%)
 create mode 100644 nwfilter_binding_wrapper.h

diff --git a/nwfilter_binding.go b/nwfilter_binding.go
index 256ac19..daab598 100644
--- a/nwfilter_binding.go
+++ b/nwfilter_binding.go
@@ -31,6 +31,7 @@ package libvirt
 #include 
 #include 
 #include "nwfilter_binding_compat.h"
+#include "nwfilter_binding_wrapper.h"
 */
 import "C"
 
diff --git a/nwfilter_binding_compat.h b/nwfilter_binding_compat.h
index 4aa138d..1d6fd16 100644
--- a/nwfilter_binding_compat.h
+++ b/nwfilter_binding_compat.h
@@ -30,15 +30,4 @@
 typedef struct _virNWFilterBinding *virNWFilterBindingPtr;
 #endif
 
-const char *virNWFilterBindingGetPortDevWrapper(virNWFilterBindingPtr binding);
-const char *virNWFilterBindingGetFilterNameWrapper(virNWFilterBindingPtr 
binding);
-
-char *virNWFilterBindingGetXMLDescWrapper(virNWFilterBindingPtr binding,
-unsigned int flags);
-
-int virNWFilterBindingDeleteWrapper(virNWFilterBindingPtr binding);
-int virNWFilterBindingRefWrapper(virNWFilterBindingPtr binding);
-int virNWFilterBindingFreeWrapper(virNWFilterBindingPtr binding);
-
-
 #endif /* LIBVIRT_GO_NWFILTER_BINDING_COMPAT_H__ */
diff --git a/nwfilter_binding_compat.go b/nwfilter_binding_wrapper.go
similarity index 98%
rename from nwfilter_binding_compat.go
rename to nwfilter_binding_wrapper.go
index 37fcce0..4d0e086 100644
--- a/nwfilter_binding_compat.go
+++ b/nwfilter_binding_wrapper.go
@@ -30,6 +30,7 @@ package libvirt
 #include 
 #include 
 #include "nwfilter_binding_compat.h"
+#include "nwfilter_binding_wrapper.h"
 
 const char *virNWFilterBindingGetPortDevWrapper(virNWFilterBindingPtr binding)
 {
diff --git a/nwfilter_binding_wrapper.h b/nwfilter_binding_wrapper.h
new file mode 100644
index 000..be09dca
--- /dev/null
+++ b/nwfilter_binding_wrapper.h
@@ -0,0 +1,38 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ */
+
+#ifndef LIBVIRT_GO_NWFILTER_BINDING_WRAPPER_H__
+#define LIBVIRT_GO_NWFILTER_BINDING_WRAPPER_H__
+
+const char *virNWFilterBindingGetPortDevWrapper(virNWFilterBindingPtr binding);
+const char *virNWFilterBindingGetFilterNameWrapper(virNWFilterBindingPtr 
binding);
+char *virNWFilterBindingGetXMLDescWrapper(virNWFilterBindingPtr binding,
+unsigned int flags);
+int virNWFilterBindingDeleteWrapper(virNWFilterBindingPtr binding);
+int virNWFilterBindingRefWrapper(virNWFilterBindingPtr binding);
+int virNWFilterBindingFreeWrapper(virNWFilterBindingPtr binding);
+
+
+#endif /* LIBVIRT_GO_NWFILTER_BINDING_WRAPPER_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 06/37] connect: move wrapper functions out of compat header

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 connect_compat.go  | 206 -
 connect_compat.h   |  81 --
 connect_wrapper.go | 172 +
 connect_wrapper.h  |  75 +
 4 files changed, 247 insertions(+), 287 deletions(-)
 delete mode 100644 connect_compat.go

diff --git a/connect_compat.go b/connect_compat.go
deleted file mode 100644
index 1ecbef2..000
--- a/connect_compat.go
+++ /dev/null
@@ -1,206 +0,0 @@
-/*
- * This file is part of the libvirt-go project
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * Copyright (c) 2013 Alex Zorin
- * Copyright (C) 2016 Red Hat, Inc.
- *
- */
-
-package libvirt
-
-/*
-#cgo pkg-config: libvirt
-#include 
-#include 
-#include "connect_compat.h"
-
-int virNodeGetFreePagesWrapper(virConnectPtr conn,
- unsigned int npages,
- unsigned int *pages,
- int startcell,
- unsigned int cellcount,
- unsigned long long *counts,
- unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 1002006
-assert(0); // Caller should have checked version
-#else
-return virNodeGetFreePages(conn, npages, pages, startcell, cellcount, 
counts, flags);
-#endif
-}
-
-char * virConnectGetDomainCapabilitiesWrapper(virConnectPtr conn,
-const char *emulatorbin,
-const char *arch,
-const char *machine,
-const char *virttype,
-unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 1002007
-assert(0); // Caller should have checked version
-#else
-return virConnectGetDomainCapabilities(conn, emulatorbin, arch, machine, 
virttype, flags);
-#endif
-}
-
-int virConnectGetAllDomainStatsWrapper(virConnectPtr conn,
- unsigned int stats,
- virDomainStatsRecordPtr **retStats,
- unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 1002008
-assert(0); // Caller should have checked version
-#else
-return virConnectGetAllDomainStats(conn, stats, retStats, flags);
-#endif
-}
-
-int virDomainListGetStatsWrapper(virDomainPtr *doms,
-   unsigned int stats,
-   virDomainStatsRecordPtr **retStats,
-   unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 1002008
-assert(0); // Caller should have checked version
-#else
-return virDomainListGetStats(doms, stats, retStats, flags);
-#endif
-}
-
-void virDomainStatsRecordListFreeWrapper(virDomainStatsRecordPtr *stats)
-{
-}
-
-int virNodeAllocPagesWrapper(virConnectPtr conn,
-   unsigned int npages,
-   unsigned int *pageSizes,
-   unsigned long long *pageCounts,
-   int startCell,
-   unsigned int cellCount,
-   unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 1002009
-assert(0); // Caller should have checked version
-#else
-return virNodeAllocPages(conn, npages, pageSizes, pageCounts, startCell, 
cellCount, flags);
-#endif
-}
-
-
-virDomainPtr virDomainDefineXMLFlagsWrapper(virConnectPtr conn,
-  const char *xml,
-  unsigned int flags)
-{
-#if LIBVIR_VERSION_NUMBER < 1002012
-assert(0); // Caller should have checked version
-#else
-return virDomainDefineXMLFlags(conn, xml, flags);
-#endif
-}
-
-virStoragePoolPtr virStoragePoolLookupByTargetPathWrapper(virConnectPtr conn,
-

[libvirt] [go PATCH 05/37] Change "Compat" suffix to "Wrapper" to have standard naming scheme

2018-07-16 Thread Daniel P . Berrangé
There are many reasons why we need to create wrappers around the native
libvirt public APIs, and multiple reasons may apply to the same
function. Using a standard "Wrapper" suffix will clarify it.

Signed-off-by: Daniel P. Berrangé 
---
 api_test.go|  2 +-
 connect.go | 28 -
 connect_compat.go  | 28 -
 connect_compat.h   | 28 -
 domain.go  | 64 +++---
 domain_compat.go   | 62 ++--
 domain_compat.h| 62 ++--
 lxc.go |  2 +-
 lxc_compat.go  |  2 +-
 lxc_compat.h   |  2 +-
 network.go |  4 +--
 network_compat.go  |  6 ++--
 network_compat.h   |  6 ++--
 network_events.go  |  2 +-
 node_device_compat.go  |  2 +-
 node_device_compat.h   |  2 +-
 node_device_events.go  |  2 +-
 nwfilter_binding.go| 12 +++
 nwfilter_binding_compat.go | 12 +++
 nwfilter_binding_compat.h  | 12 +++
 qemu.go|  2 +-
 qemu_compat.go |  2 +-
 qemu_compat.h  |  2 +-
 secret_compat.go   |  2 +-
 secret_compat.h|  2 +-
 secret_events.go   |  2 +-
 storage_pool_compat.go |  2 +-
 storage_pool_compat.h  |  2 +-
 storage_pool_events.go |  2 +-
 storage_volume.go  |  2 +-
 storage_volume_compat.go   |  2 +-
 storage_volume_compat.h|  2 +-
 stream.go  |  6 ++--
 stream_compat.go   |  6 ++--
 stream_compat.h|  6 ++--
 35 files changed, 191 insertions(+), 191 deletions(-)

diff --git a/api_test.go b/api_test.go
index ad94335..a16a1eb 100644
--- a/api_test.go
+++ b/api_test.go
@@ -394,7 +394,7 @@ func ProcessFile(path string) []string {
 
defer file.Close()
 
-   re, err := 
regexp.Compile("C\\.((vir|VIR|LIBVIR)[a-zA-Z0-9_]+?)(Compat|Wrapper)?\\b")
+   re, err := 
regexp.Compile("C\\.((vir|VIR|LIBVIR)[a-zA-Z0-9_]+?)(Wrapper)?\\b")
if err != nil {
panic(err)
}
diff --git a/connect.go b/connect.go
index 793e3c4..98ab7ab 100644
--- a/connect.go
+++ b/connect.go
@@ -833,7 +833,7 @@ func (c *Connect) DomainDefineXMLFlags(xmlConfig string, 
flags DomainDefineFlags
}
cXml := C.CString(string(xmlConfig))
defer C.free(unsafe.Pointer(cXml))
-   ptr := C.virDomainDefineXMLFlagsCompat(c.ptr, cXml, C.uint(flags))
+   ptr := C.virDomainDefineXMLFlagsWrapper(c.ptr, cXml, C.uint(flags))
if ptr == nil {
return nil, GetLastError()
}
@@ -1210,7 +1210,7 @@ func (c *Connect) LookupStoragePoolByTargetPath(path 
string) (*StoragePool, erro
}
cPath := C.CString(path)
defer C.free(unsafe.Pointer(cPath))
-   ptr := C.virStoragePoolLookupByTargetPathCompat(c.ptr, cPath)
+   ptr := C.virStoragePoolLookupByTargetPathWrapper(c.ptr, cPath)
if ptr == nil {
return nil, GetLastError()
}
@@ -1274,7 +1274,7 @@ func (c *Connect) LookupNWFilterBindingByPortDev(name 
string) (*NWFilterBinding,
}
cName := C.CString(name)
defer C.free(unsafe.Pointer(cName))
-   ptr := C.virNWFilterBindingLookupByPortDevCompat(c.ptr, cName)
+   ptr := C.virNWFilterBindingLookupByPortDevWrapper(c.ptr, cName)
if ptr == nil {
return nil, GetLastError()
}
@@ -1478,7 +1478,7 @@ func (c *Connect) ListAllNWFilterBindings(flags uint32) 
([]NWFilterBinding, erro
if C.LIBVIR_VERSION_NUMBER < 4005000 {
return []NWFilterBinding{}, 
GetNotImplementedError("virConnectListAllNWFilterBindings")
}
-   numNWFilters := C.virConnectListAllNWFilterBindingsCompat(c.ptr, 
(**C.virNWFilterBindingPtr)(&cList), C.uint(flags))
+   numNWFilters := C.virConnectListAllNWFilterBindingsWrapper(c.ptr, 
(**C.virNWFilterBindingPtr)(&cList), C.uint(flags))
if numNWFilters == -1 {
return nil, GetLastError()
}
@@ -1601,7 +1601,7 @@ func (c *Connect) AllocPages(pageSizes map[int]int64, 
startCell int, cellCount u
i++
}
 
-   ret := C.virNodeAllocPagesCompat(c.ptr, C.uint(len(pageSizes)), 
(*C.uint)(unsafe.Pointer(&cpages)),
+   ret := C.virNodeAllocPagesWrapper(c.ptr, C.uint(len(pageSizes)), 
(*C.uint)(unsafe.Pointer(&cpages)),
(*C.ulonglong)(unsafe.Pointer(&ccounts)), C.int(startCell), 
C.uint(cellCount), C.uint(flags))
if ret == -1 {
return 0, GetLastError()
@@ -1730,7 +1730,7 @@ func (c *Connect) GetFreePages(pageSizes []uint64, 
startCell int, maxCells uint,
cpageSizes[i] = C.uint(pageSizes[i])
}
 
-   ret := C.virNodeGetFreePagesCompat(c.ptr, C.uint(len(pageSizes)), 
(*C.uint)(unsafe.Pointer(&cpageSizes)), C.int(startCell),
+   ret := C.virNodeGetFreePagesWrapper(c

[libvirt] [go PATCH 02/37] storage volume: add missin blank line

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 storage_volume.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/storage_volume.go b/storage_volume.go
index 22dd284..739cdb2 100644
--- a/storage_volume.go
+++ b/storage_volume.go
@@ -239,6 +239,7 @@ func (v *StorageVol) Wipe(flags uint32) error {
}
return nil
 }
+
 // See also 
https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolWipePattern
 func (v *StorageVol) WipePattern(algorithm StorageVolWipeAlgorithm, flags 
uint32) error {
result := C.virStorageVolWipePattern(v.ptr, C.uint(algorithm), 
C.uint(flags))
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 04/37] Use "Wrapper" or "Helper" as suffix for C functions

2018-07-16 Thread Daniel P . Berrangé
Instead of using "_cgo" for multiple scenarios, use "Wrapper" as suffix
where a libvirt API is being wrapped, and "Helper" as a suffix where we
are implementing a helper function (eg an event callback).

Signed-off-by: Daniel P. Berrangé 
---
 api_test.go|   2 +-
 callbacks.go   |   4 +-
 callbacks_wrapper.go   |   2 +-
 callbacks_wrapper.h|   2 +-
 connect.go |   6 +-
 connect_wrapper.go |  10 ++--
 connect_wrapper.h  |   4 +-
 domain_events.go   | 100 -
 domain_events_wrapper.go   |  50 -
 domain_events_wrapper.h|  48 
 events.go  |   6 +-
 events_wrapper.go  |   6 +-
 events_wrapper.h   |   6 +-
 network_events.go  |   4 +-
 network_events_wrapper.go  |   6 +-
 network_events_wrapper.h   |   4 +-
 node_device_events.go  |   8 +--
 node_device_events_wrapper.go  |   8 +--
 node_device_events_wrapper.h   |   6 +-
 qemu.go|   4 +-
 qemu_wrapper.go|   6 +-
 qemu_wrapper.h |   4 +-
 secret_events.go   |   8 +--
 secret_events_wrapper.go   |   8 +--
 secret_events_wrapper.h|   6 +-
 storage_pool_events.go |   8 +--
 storage_pool_events_wrapper.go |   8 +--
 storage_pool_events_wrapper.h  |   6 +-
 stream.go  |  10 ++--
 stream_wrapper.go  |  10 ++--
 stream_wrapper.h   |  10 ++--
 31 files changed, 185 insertions(+), 185 deletions(-)

diff --git a/api_test.go b/api_test.go
index 06938a6..ad94335 100644
--- a/api_test.go
+++ b/api_test.go
@@ -394,7 +394,7 @@ func ProcessFile(path string) []string {
 
defer file.Close()
 
-   re, err := 
regexp.Compile("C\\.((vir|VIR|LIBVIR)[a-zA-Z0-9_]+?)(Compat|_cgo)?\\b")
+   re, err := 
regexp.Compile("C\\.((vir|VIR|LIBVIR)[a-zA-Z0-9_]+?)(Compat|Wrapper)?\\b")
if err != nil {
panic(err)
}
diff --git a/callbacks.go b/callbacks.go
index 05a755b..7b2d11b 100644
--- a/callbacks.go
+++ b/callbacks.go
@@ -40,13 +40,13 @@ package libvirt
 //
 // - Create a CGO function similar to the above function but with the
 //   appropriate signature to be registered as a callback in C code
-//   (connErrCallback_cgo). Notably, it will have a void* argument
+//   (connErrCallbackHelper). Notably, it will have a void* argument
 //   that should be cast to long to retrieve the callback ID. It
 //   should be just a thin wrapper to transform the opaque argument to
 //   a callback ID.
 //
 // - Create a CGO function which will be a wrapper around the C
-//   function to register the callback (virConnSetErrorFunc_cgo). Its
+//   function to register the callback (virConnSetErrorFuncWrapper). Its
 //   only role is to transform a callback ID (long) to an opaque (void*)
 //   and call the C function.
 //
diff --git a/callbacks_wrapper.go b/callbacks_wrapper.go
index 967aaab..64caf69 100644
--- a/callbacks_wrapper.go
+++ b/callbacks_wrapper.go
@@ -33,7 +33,7 @@ package libvirt
 #include "callbacks_wrapper.h"
 
 extern void freeCallbackId(long);
-void freeGoCallback_cgo(void* goCallbackId) {
+void freeGoCallbackHelper(void* goCallbackId) {
freeCallbackId((long)goCallbackId);
 }
 
diff --git a/callbacks_wrapper.h b/callbacks_wrapper.h
index f37aba4..4b91f7a 100644
--- a/callbacks_wrapper.h
+++ b/callbacks_wrapper.h
@@ -27,6 +27,6 @@
 #ifndef LIBVIRT_GO_CALLBACKS_WRAPPER_H__
 #define LIBVIRT_GO_CALLBACKS_WRAPPER_H__
 
-void freeGoCallback_cgo(void* goCallbackId);
+void freeGoCallbackHelper(void* goCallbackId);
 
 #endif /* LIBVIRT_GO_CALLBACKS_WRAPPER_H__ */
diff --git a/connect.go b/connect.go
index 5c6d6ff..793e3c4 100644
--- a/connect.go
+++ b/connect.go
@@ -452,8 +452,8 @@ type CloseCallback func(conn *Connect, reason 
ConnectCloseReason)
 func (c *Connect) RegisterCloseCallback(callback CloseCallback) error {
c.UnregisterCloseCallback()
goCallbackId := registerCallbackId(callback)
-   callbackPtr := unsafe.Pointer(C.closeCallback_cgo)
-   res := C.virConnectRegisterCloseCallback_cgo(c.ptr, 
C.virConnectCloseFunc(callbackPtr), C.long(goCallbackId))
+   callbackPtr := unsafe.Pointer(C.closeCallbackHelper)
+   res := C.virConnectRegisterCloseCallbackHelper(c.ptr, 
C.virConnectCloseFunc(callbackPtr), C.long(goCallbackId))
if res != 0 {
freeCallbackId(goCallbackId)
return GetLastError()
@@ -469,7 +469,7 @@ func (c *Connect) UnregisterCloseCallback() error {
if connData.closeCallbackId == nil {
return nil
}
-   callbackPtr := unsafe.Pointer(C.closeCallback_cgo)
+   callbackPtr := unsafe.Pointer(C.closeCallbackHelper)
res := C.virConnectUnregisterCloseCallback(c.ptr, 
C.virConnectCloseFunc(callbackPtr))
if res != 0 {
return Get

[libvirt] [go PATCH 01/37] error: add helper for converting libvirt to go error objects

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 error.go | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/error.go b/error.go
index bee8b70..b8e2900 100644
--- a/error.go
+++ b/error.go
@@ -583,6 +583,17 @@ func (err Error) Error() string {
err.Code, err.Domain, err.Message)
 }
 
+func makeError(err *C.virError) Error {
+   ret := Error{
+   Code:ErrorNumber(err.code),
+   Domain:  ErrorDomain(err.domain),
+   Message: C.GoString(err.message),
+   Level:   ErrorLevel(err.level),
+   }
+   C.virResetError(err)
+   return ret
+}
+
 func GetLastError() Error {
err := C.virGetLastError()
if err == nil {
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [go PATCH 00/37] Fix error reporting thread safety wrt goroutine rescheduling

2018-07-16 Thread Daniel P . Berrangé
The Libvirt C API provides the virGetLastError() function to let callers
aquire the error information after a function fails. This function uses
a thread local, and so must be called in the same OS thread as the
function that  failed originally.
eg you could call

char *xml = virDomainGetXMLDesc(dom);
if (!xml) {
virErrorPtr err = virGetLastError();
do stuff with err...
}

This is all fine, but there is a subtle problem that was overlooked when
the Go bindings were first created. Specifically a native C API call is
a goroutine re-scheduling point. So when the Go code does

var xml = C.virDomainGetXMLDesc(dom);
if (xml == nil) {
C.virErrorPtr err = C.virGetLastError();
do stuff with err...
}

All that code runs in the same goroutine, but at the call entry to
C.virGetLastError, the goroutine might get switched to a different
OS level thread. As a result virGetLastError() will return either no
error at all, or an error from a completely different libvirt API call.

We need to prevent the OS level thread being changed in between the call
to the real function and the virGetLastError() function.

Naively you might think we could put a LockOSThread() / UnlockOSThread()
call around this block of Go code, but that is a very bad idea in
reality. Until Go 1.10, the LockOSThread() calls did not ref count, so
if some other code has already locked the thread, when libvirt called
UnlockOSThread it could do bad things. In addition, after calling
UnlockOSThread() the Go runtime doesn't trust the OS thread state
anymore, so will terminate the thread and spawn a new one. IOW using
LockOSThread() would mean every single libvirt API call would create and
destroy a new thread which is horrible for performance.

Thus this patch series takes a different approach. We create a wrapper
function for every C API exposed by libvirt, that has a 'virErrorPtr'
parameter. So the Go code can do

 var C.virErrorPtr err
 var xml = C.virDomainGetXMLDescWrapper(dom, &err)
 if (xml == nil) {
 ...do stuff with err...
 }

The wrapper function is responsible for calling virGetLastError() and
since this is C code, we're guaranteed its all in the same OS level
thread.

Daniel P. Berrangé (37):
  error: add helper for converting libvirt to go error objects
  storage volume: add missin blank line
  Rename *cfuncs.{go,h} to *wrapper.{go,h}
  Use "Wrapper" or "Helper" as suffix for C functions
  Change "Compat" suffix to "Wrapper" to have standard naming scheme
  connect: move wrapper functions out of compat header
  network: move wrapper functions out of compat header
  nwfilter binding: move wrapper functions out of compat header
  node device: move wrapper functions out of compat header
  secret: move wrapper functions out of compat header
  stream: move wrapper functions out of compat header
  storage volume: move wrapper functions out of compat header
  storage pool: move wrapper functions out of compat header
  qemu: move wrapper functions out of compat header
  lxc: move wrapper functions out of compat header
  domain: move wrapper functions out of compat header
  make the XXX_wrapper.h header files self-contained
  Add XXX_wrapper.{h,go} for every remaining file
  Standardize formatting in all wrapper headers
  storage vol: fix error reporting thread safety
  storage pool: fix error reporting thread safety
  stream: fix error reporting thread safety
  secret: fix error reporting thread safety
  nwfilter: fix error reporting thread safety
  nwfilter binding: fix error reporting thread safety
  node device: fix error reporting thread safety
  network: fix error reporting thread safety
  interface: fix error reporting thread safety
  domain snapshot: fix error reporting thread safety
  connect: fix error reporting thread safety
  domain: fix error reporting thread safety
  qemu: fix error reporting thread safety
  lxc: fix error reporting thread safety
  events: fix error reporting thread safety
  error: remove GetLastError() function
  error: make GetNotImplementedError private
  connect: add missing references on domain object in stats records

 api_test.go   |5 +-
 callbacks.go  |4 +-
 callbacks_cfuncs.go => callbacks_wrapper.go   |6 +-
 callbacks_cfuncs.h => callbacks_wrapper.h |8 +-
 connect.go|  741 +++---
 connect_cfuncs.h  |   34 -
 connect_compat.go |  206 --
 connect_compat.h  |   81 -
 connect_wrapper.go| 1766 +
 connect_wrapper.h |  730 ++
 domain.go | 1083 
 domain_compat.go  |  384 ---
 domain_compat.h   |  141 -
 domain_events.go  |  235 +-
 domain_eve

[libvirt] [go PATCH 03/37] Rename *cfuncs.{go, h} to *wrapper.{go, h}

2018-07-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 api_test.go | 2 +-
 callbacks_cfuncs.go => callbacks_wrapper.go | 2 +-
 callbacks_cfuncs.h => callbacks_wrapper.h   | 6 +++---
 connect.go  | 2 +-
 connect_cfuncs.go => connect_wrapper.go | 4 ++--
 connect_cfuncs.h => connect_wrapper.h   | 6 +++---
 domain_events.go| 2 +-
 domain_events_cfuncs.go => domain_events_wrapper.go | 4 ++--
 domain_events_cfuncs.h => domain_events_wrapper.h   | 6 +++---
 events.go   | 2 +-
 events_cfuncs.go => events_wrapper.go   | 2 +-
 events_cfuncs.h => events_wrapper.h | 6 +++---
 network_events.go   | 2 +-
 network_events_cfuncs.go => network_events_wrapper.go   | 4 ++--
 network_events_cfuncs.h => network_events_wrapper.h | 6 +++---
 node_device_events.go   | 2 +-
 ...device_events_cfuncs.go => node_device_events_wrapper.go | 4 ++--
 node_device_events_cfuncs.h => node_device_events_wrapper.h | 6 +++---
 qemu.go | 2 +-
 qemu_cfuncs.go => qemu_wrapper.go   | 4 ++--
 qemu_cfuncs.h => qemu_wrapper.h | 6 +++---
 secret_events.go| 2 +-
 secret_events_cfuncs.go => secret_events_wrapper.go | 4 ++--
 secret_events_cfuncs.h => secret_events_wrapper.h   | 6 +++---
 storage_pool_events.go  | 2 +-
 ..._pool_events_cfuncs.go => storage_pool_events_wrapper.go | 4 ++--
 ...ge_pool_events_cfuncs.h => storage_pool_events_wrapper.h | 6 +++---
 stream.go   | 2 +-
 stream_cfuncs.go => stream_wrapper.go   | 2 +-
 stream_cfuncs.h => stream_wrapper.h | 6 +++---
 30 files changed, 57 insertions(+), 57 deletions(-)
 rename callbacks_cfuncs.go => callbacks_wrapper.go (97%)
 rename callbacks_cfuncs.h => callbacks_wrapper.h (90%)
 rename connect_cfuncs.go => connect_wrapper.go (97%)
 rename connect_cfuncs.h => connect_wrapper.h (92%)
 rename domain_events_cfuncs.go => domain_events_wrapper.go (99%)
 rename domain_events_cfuncs.h => domain_events_wrapper.h (97%)
 rename events_cfuncs.go => events_wrapper.go (99%)
 rename events_cfuncs.h => events_wrapper.h (93%)
 rename network_events_cfuncs.go => network_events_wrapper.go (97%)
 rename network_events_cfuncs.h => network_events_wrapper.h (91%)
 rename node_device_events_cfuncs.go => node_device_events_wrapper.go (97%)
 rename node_device_events_cfuncs.h => node_device_events_wrapper.h (91%)
 rename qemu_cfuncs.go => qemu_wrapper.go (97%)
 rename qemu_cfuncs.h => qemu_wrapper.h (91%)
 rename secret_events_cfuncs.go => secret_events_wrapper.go (97%)
 rename secret_events_cfuncs.h => secret_events_wrapper.h (92%)
 rename storage_pool_events_cfuncs.go => storage_pool_events_wrapper.go (97%)
 rename storage_pool_events_cfuncs.h => storage_pool_events_wrapper.h (91%)
 rename stream_cfuncs.go => stream_wrapper.go (99%)
 rename stream_cfuncs.h => stream_wrapper.h (93%)

diff --git a/api_test.go b/api_test.go
index 53632df..06938a6 100644
--- a/api_test.go
+++ b/api_test.go
@@ -52,7 +52,7 @@ var (
/* Obsolete we use virConnectDomainEventRegisterAny instead */
"virConnectDomainEventRegister",
 
-   /* Wrapped in connect_cfuncs.go instead */
+   /* Wrapped in connect_wrapper.go instead */
"virConnectOpenAuth",
"virConnectRegisterCloseCallback",
 
diff --git a/callbacks_cfuncs.go b/callbacks_wrapper.go
similarity index 97%
rename from callbacks_cfuncs.go
rename to callbacks_wrapper.go
index 2cef0e4..967aaab 100644
--- a/callbacks_cfuncs.go
+++ b/callbacks_wrapper.go
@@ -30,7 +30,7 @@ package libvirt
 #cgo pkg-config: libvirt
 #include 
 #include 
-#include "callbacks_cfuncs.h"
+#include "callbacks_wrapper.h"
 
 extern void freeCallbackId(long);
 void freeGoCallback_cgo(void* goCallbackId) {
diff --git a/callbacks_cfuncs.h b/callbacks_wrapper.h
similarity index 90%
rename from callbacks_cfuncs.h
rename to callbacks_wrapper.h
index 689a777..f37aba4 100644
--- a/callbacks_cfuncs.h
+++ b/callbacks_wrapper.h
@@ -24,9 +24,9 @@
  *
  */
 
-#ifndef LIBVIRT_GO_CALLBACKS_CFUNCS_H__
-#define LIBVIRT_GO_CALLBACKS_CFUNCS_H__
+#ifndef LIBVIRT_GO_CALLBACKS_WRAPPER_H__
+#define LIBVIRT_GO_CALLBACKS_WRAPPER_H__
 
 void freeGoCallback_cgo(void* goCallbackId);
 
-#endif /* LIBVIRT_GO_CALLBACKS_CFUNCS_H__ */
+#endif /* LIBVIRT_GO_CALLBACKS_WRAPPER_H__ */
diff --git a/connect.go b/connect.go
index c608658..5c6d6ff 100644
--- a/connect.go
+++ b/connect.go
@@ -40,7 +40,7 @@ i

[libvirt] [PATCH] qemu: qemu_driver: Fix setting global_period cputune element

2018-07-16 Thread Katerina Koukiou
When VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD is matched "cputune.global_period"
should be updated and not "cputune.period".

Signed-off-by: Katerina Koukiou 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5de9aaefbb..8fae46370e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10556,7 +10556,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
 }
 
 if (persistentDef)
-persistentDefCopy->cputune.period = value_ul;
+persistentDefCopy->cputune.global_period = value_ul;
 
 } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA)) {
 SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA,
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] New virsh feature: domif-setlink --domain --interface --state completer

2018-07-16 Thread Peter Krempa
On Mon, Jul 16, 2018 at 13:55:34 +0100, Daniel Berrange wrote:
> On Mon, Jul 16, 2018 at 02:47:40PM +0200, Peter Krempa wrote:
> > On Thu, Jul 12, 2018 at 10:15:22 +0200, Simon Kobyda wrote:
> > >   After you go through command mentioned above, completer finds
> > >   what state the device is currently in, and suggests an opposite state.
> > > ---
> > >  tools/virsh-completer.c | 73 -
> > >  tools/virsh-completer.h |  4 +++
> > >  tools/virsh-domain.c|  1 +
> > >  3 files changed, 77 insertions(+), 1 deletion(-)
> > 
> > Looks like your date/time is wrong.
> 
> It is my bad - this message was blocked in the moderation queue since last
> week, and I have only just approved it.

Hmm, yeah. It was weird because there were other patches from him. Also
looks like this was already pushed in version 2


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] New virsh feature: domif-setlink --domain --interface --state completer

2018-07-16 Thread Daniel P . Berrangé
On Mon, Jul 16, 2018 at 02:47:40PM +0200, Peter Krempa wrote:
> On Thu, Jul 12, 2018 at 10:15:22 +0200, Simon Kobyda wrote:
> > After you go through command mentioned above, completer finds
> > what state the device is currently in, and suggests an opposite state.
> > ---
> >  tools/virsh-completer.c | 73 -
> >  tools/virsh-completer.h |  4 +++
> >  tools/virsh-domain.c|  1 +
> >  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> Looks like your date/time is wrong.

It is my bad - this message was blocked in the moderation queue since last
week, and I have only just approved it.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] New virsh feature: domif-setlink --domain --interface --state completer

2018-07-16 Thread Peter Krempa
On Thu, Jul 12, 2018 at 10:15:22 +0200, Simon Kobyda wrote:
>   After you go through command mentioned above, completer finds
>   what state the device is currently in, and suggests an opposite state.
> ---
>  tools/virsh-completer.c | 73 -
>  tools/virsh-completer.h |  4 +++
>  tools/virsh-domain.c|  1 +
>  3 files changed, 77 insertions(+), 1 deletion(-)

Looks like your date/time is wrong.

> 
> diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> index 2327e08340..e32fd82211 100644
> --- a/tools/virsh-completer.c
> +++ b/tools/virsh-completer.c
> @@ -32,6 +32,7 @@
>  #include "internal.h"
>  #include "virutil.h"
>  #include "viralloc.h"
> +#include "virmacaddr.h"
>  #include "virstring.h"
>  #include "virxml.h"
>  
> @@ -750,7 +751,6 @@ virshDomainEventNameCompleter(vshControl *ctl 
> ATTRIBUTE_UNUSED,
>  return NULL;
>  }
>  
> -

We tend to keep two lines between functions.

>  char **
>  virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
>  const vshCmd *cmd ATTRIBUTE_UNUSED,
> @@ -776,6 +776,77 @@ virshPoolEventNameCompleter(vshControl *ctl 
> ATTRIBUTE_UNUSED,
>  return NULL;
>  }
>  
> +char **
> +virshDomainInterfaceStateCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
> +const vshCmd *cmd ATTRIBUTE_UNUSED,
> +unsigned int flags)
> +{
> +virshControlPtr priv = ctl->privData;
> +const char *iface = NULL;
> +char **ret = NULL;
> +xmlDocPtr xml = NULL;
> +virMacAddr macaddr;
> +char *state = NULL;
> +char *xpath = NULL;
> +char macstr[18] = "";
> +xmlXPathContextPtr ctxt = NULL;
> +xmlNodePtr *interfaces = NULL;
> +int ninterfaces;
> +
> +virCheckFlags(0, NULL);
> +
> +if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> +return NULL;
> +
> +if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0)
> +goto cleanup;
> +
> +if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
> +goto cleanup;
> +
> +/* normalize the mac addr */
> +if (virMacAddrParse(iface, &macaddr) == 0)
> +virMacAddrFormat(&macaddr, macstr);
> +
> +if (virAsprintf(&xpath, "/domain/devices/interface[(mac/@address = '%s') 
> or "
> +"  (target/@dev = 
> '%s')]",
> +   macstr, iface) < 0)
> +goto cleanup;
> +
> +if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0)
> +goto cleanup;
> +
> +if (ninterfaces != 1)
> +goto cleanup;
> +
> +ctxt->node = interfaces[0];
> +
> +if (VIR_ALLOC_N(ret, 2) < 0)
> +goto error;
> +
> +if ((state = virXPathString("string(./link/@state)", ctxt)) &&
> +STREQ(state, "down")) {
> +if (VIR_STRDUP(ret[0], "up") < 0)
> +goto error;
> +} else {
> +if (VIR_STRDUP(ret[0], "down") < 0)
> +goto error;
> +}
> +
> + cleanup:
> +VIR_FREE(state);
> +VIR_FREE(interfaces);
> +VIR_FREE(xpath);
> +xmlXPathFreeContext(ctxt);
> +xmlFreeDoc(xml);
> +return ret;
> +
> + error:
> +virStringListFree(ret);
> +ret = NULL;
> +goto cleanup;
> +}

This is an awful lot of code for a not very useful feature. I'd just
complete both up and down and don't really care what the current state
is.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] New virsh feature: domif-setlink --domain --interface --state completer

2018-07-16 Thread Simon Kobyda
After you go through command mentioned above, completer finds
what state the device is currently in, and suggests an opposite state.
---
 tools/virsh-completer.c | 73 -
 tools/virsh-completer.h |  4 +++
 tools/virsh-domain.c|  1 +
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 2327e08340..e32fd82211 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -32,6 +32,7 @@
 #include "internal.h"
 #include "virutil.h"
 #include "viralloc.h"
+#include "virmacaddr.h"
 #include "virstring.h"
 #include "virxml.h"
 
@@ -750,7 +751,6 @@ virshDomainEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
 return NULL;
 }
 
-
 char **
 virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
 const vshCmd *cmd ATTRIBUTE_UNUSED,
@@ -776,6 +776,77 @@ virshPoolEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
 return NULL;
 }
 
+char **
+virshDomainInterfaceStateCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
+const vshCmd *cmd ATTRIBUTE_UNUSED,
+unsigned int flags)
+{
+virshControlPtr priv = ctl->privData;
+const char *iface = NULL;
+char **ret = NULL;
+xmlDocPtr xml = NULL;
+virMacAddr macaddr;
+char *state = NULL;
+char *xpath = NULL;
+char macstr[18] = "";
+xmlXPathContextPtr ctxt = NULL;
+xmlNodePtr *interfaces = NULL;
+int ninterfaces;
+
+virCheckFlags(0, NULL);
+
+if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+return NULL;
+
+if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0)
+goto cleanup;
+
+if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
+goto cleanup;
+
+/* normalize the mac addr */
+if (virMacAddrParse(iface, &macaddr) == 0)
+virMacAddrFormat(&macaddr, macstr);
+
+if (virAsprintf(&xpath, "/domain/devices/interface[(mac/@address = '%s') 
or "
+"  (target/@dev = '%s')]",
+   macstr, iface) < 0)
+goto cleanup;
+
+if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0)
+goto cleanup;
+
+if (ninterfaces != 1)
+goto cleanup;
+
+ctxt->node = interfaces[0];
+
+if (VIR_ALLOC_N(ret, 2) < 0)
+goto error;
+
+if ((state = virXPathString("string(./link/@state)", ctxt)) &&
+STREQ(state, "down")) {
+if (VIR_STRDUP(ret[0], "up") < 0)
+goto error;
+} else {
+if (VIR_STRDUP(ret[0], "down") < 0)
+goto error;
+}
+
+ cleanup:
+VIR_FREE(state);
+VIR_FREE(interfaces);
+VIR_FREE(xpath);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+return ret;
+
+ error:
+virStringListFree(ret);
+ret = NULL;
+goto cleanup;
+}
+
 
 char **
 virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 1c14bb2a54..b4fd2a86c6 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -94,6 +94,10 @@ char ** virshPoolEventNameCompleter(vshControl *ctl,
 const vshCmd *cmd,
 unsigned int flags);
 
+char ** virshDomainInterfaceStateCompleter(vshControl *ctl,
+   const vshCmd *cmd,
+   unsigned int flags);
+
 char ** virshNodedevEventNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3959b5475b..8adec1d9b1 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3064,6 +3064,7 @@ static const vshCmdOptDef opts_domif_setlink[] = {
 {.name = "state",
  .type = VSH_OT_DATA,
  .flags = VSH_OFLAG_REQ,
+ .completer = virshDomainInterfaceStateCompleter,
  .help = N_("new state of the device")
 },
 {.name = "persistent",
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] examples: Add clean-traffic-gateway into nwfilters

2018-07-16 Thread Ales Musil
This filter should be used to limit traffic between VMs
based on their MAC adddresses. The MAC address can be
set with GATEWAY_MAC and mask with GATEWAY_MAC_MASK
variable.

Signed-off-by: Ales Musil 
---
 examples/xml/nwfilter/clean-traffic-gateway.xml | 36 +
 1 file changed, 36 insertions(+)
 create mode 100644 examples/xml/nwfilter/clean-traffic-gateway.xml

diff --git a/examples/xml/nwfilter/clean-traffic-gateway.xml 
b/examples/xml/nwfilter/clean-traffic-gateway.xml
new file mode 100644
index 000..d1e7c81
--- /dev/null
+++ b/examples/xml/nwfilter/clean-traffic-gateway.xml
@@ -0,0 +1,36 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] util: Drop virArgvToString()

2018-07-16 Thread Ján Tomko

On Mon, Jul 16, 2018 at 01:06:06PM +0200, Andrea Bolognani wrote:

The last use has been removed in 026ae4933c6a.

Signed-off-by: Andrea Bolognani 
---
src/libvirt_private.syms |  1 -
src/util/virstring.c | 27 ---
src/util/virstring.h |  2 --
3 files changed, 30 deletions(-)



Beautiful diffstat.

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Drop virArgvToString()

2018-07-16 Thread Peter Krempa
On Mon, Jul 16, 2018 at 13:06:06 +0200, Andrea Bolognani wrote:
> The last use has been removed in 026ae4933c6a.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/libvirt_private.syms |  1 -
>  src/util/virstring.c | 27 ---
>  src/util/virstring.h |  2 --
>  3 files changed, 30 deletions(-)

ACK, trivial.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] util: Drop virArgvToString()

2018-07-16 Thread Andrea Bolognani
The last use has been removed in 026ae4933c6a.

Signed-off-by: Andrea Bolognani 
---
 src/libvirt_private.syms |  1 -
 src/util/virstring.c | 27 ---
 src/util/virstring.h |  2 --
 3 files changed, 30 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e688981c3e..1caecb96b6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2876,7 +2876,6 @@ virStorageFileBackendRegister;
 
 
 # util/virstring.h
-virArgvToString;
 virAsprintfInternal;
 virSkipSpaces;
 virSkipSpacesAndBackslash;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 15f367af7c..31e71d7535 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -917,33 +917,6 @@ virStringIsEmpty(const char *str)
 return str[0] == '\0';
 }
 
-char *
-virArgvToString(const char *const *argv)
-{
-int len;
-size_t i;
-char *ret, *p;
-
-for (len = 1, i = 0; argv[i]; i++)
-len += strlen(argv[i]) + 1;
-
-if (VIR_ALLOC_N(ret, len) < 0)
-return NULL;
-p = ret;
-
-for (i = 0; argv[i]; i++) {
-if (i != 0)
-*(p++) = ' ';
-
-strcpy(p, argv[i]);
-p += strlen(argv[i]);
-}
-
-*p = '\0';
-
-return ret;
-}
-
 /**
  * virStrdup:
  * @dest: where to store duplicated string
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 726e02b98c..14948fdf1c 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -65,8 +65,6 @@ char *virStringListGetFirstWithPrefix(char **strings,
   const char *prefix)
 ATTRIBUTE_NONNULL(2);
 
-char *virArgvToString(const char *const *argv);
-
 int virStrToLong_i(char const *s,
char **end_ptr,
int base,
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] util: Remove ATTRIBUTE_NONNULL from virNetDevTapInterfaceStats

2018-07-16 Thread John Ferlan
Commit id 318d54e520 altered the code to check for a NULL
first parameter, but neglected to alter the prototype.

Signed-off-by: John Ferlan 
---

 Pushed under the trivial rule and as a Coverity build breaker.

 src/util/virnetdevtap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index 727368fe95..37f9367ff4 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -94,6 +94,6 @@ int virNetDevTapCreateInBridgePort(const char *brname,
 int virNetDevTapInterfaceStats(const char *ifname,
virDomainInterfaceStatsPtr stats,
bool swapped)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+ATTRIBUTE_RETURN_CHECK;
 
 #endif /* __VIR_NETDEV_TAP_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

2018-07-16 Thread Kashyap Chamarthy
On Fri, Jul 13, 2018 at 01:35:02PM +0200, Cornelia Huck wrote:
> On Thu, 12 Jul 2018 17:47:00 +0200
> Thomas Huth  wrote:
> > On 12.07.2018 08:32, Markus Armbruster wrote:
> > > Daniel P. Berrangé  writes:  

[...]

> > > Our tool to help with getting cc: wrong less often is the MAINTAINERS
> > > file.  Could one of the libvirt developers watch changes to qemu-doc
> > > appendix "Deprecated features"?  Would putting the appendix in its own
> > > .texi help with that?  
> > 
> > Sound like a good idea to put the appendix in its own texi file. Then
> > add an "R: libvir-list" entry for that file to MAINTAINERS and we should
> > be fine (at least for the people who use the get_maintainers.pl script).
> 
> +1, like that idea
> 
> Are there other consumers of QEMU's interfaces which should be cc:ed in
> a similar way?

Perhaps starting with libvirt is fine -- as most of the open source
management applications rely on it.  (But if we do know other consumers,
then they can be trivially added.)

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 7/9] rpc: Alter virNetDaemonQuit processing

2018-07-16 Thread Peter Krempa
On Mon, Jul 16, 2018 at 10:18:59 +0100, Daniel Berrange wrote:
> On Thu, Jul 12, 2018 at 03:34:00PM -0400, John Ferlan wrote:
> > On 07/09/2018 03:11 AM, Peter Krempa wrote:
> > > On Sat, Jul 07, 2018 at 08:11:05 -0400, John Ferlan wrote:
> 
> I think the key thing to bear in mind is what is the benefit of what we
> are trying todo. ie why do we need todo a clean shutdown instead of just
> immediately exiting. The kernel will clean up all resources associated
> with the process when it exits. So we only need care about a clean shutdown
> if there is something needing cleanup that the kernel won't handle, or if
> we are trying to debug with valgrind & similar tools.
> 
> I tend to think we put way too much time into worrying about getting perfect
> clean shutdown, for little real world benefit.
> 
> IMHO, we should make a moderate effort to shutdown cleanly, but if there's
> something stuck after 15 seconds, we should just uncoditionally call exit().

I'm not really worried about the uncleannes of the shutdown but rather
desynchronisation of qemu-libvirt state. If we exit during a modify job
prior to writing the status XML to disk we might end up in an
intermediate state which will not be recognized by libvirt afterwards.

Just exit()-ing is not a systematic solution here because of the above.

In my opinion a better solution is to close the monitor connections
after some time and only in cases where we know it's safe. E.g. in QUERY
type jobs. Some modify-type jobs may get stuck as well, but only a very
few commands create this risk so annotating them and not allowing to
break the monitor there should be way better.

Currently I'm mostly worried about the 'transaction' command which is
used for snapshots, as losing track of the new files is not recoverable.

Other block jobs which modify the backing graph would be a problem
normally but since we don't track the bakcing chain fully yet it's okay
except for finishing step of the active block commit.

For the blockjobs it will be possible to work this around by the new
APIs in qemu which allow the jobs to linger until we retrieve the state.

That means that the only problematic job probably will be
'transaction'/snapshot creation.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

2018-07-16 Thread Daniel P . Berrangé
On Mon, Jul 16, 2018 at 09:32:25AM +0200, Markus Armbruster wrote:
> Consumers of QEMU need to track feature deprecation.  Keeping
> deprecation documentation in its own file helps in two small ways:
> 
> * You can track changes the easy and obvious way, with git-log.
>   Before, you had to resort to more complex gittery like "git-log
>   --oneline -L '/@node Deprecated features/,/@node Supported build
>   platforms/:qemu-doc.texi'"
> 
> * It lets us use MAINTAINERS to copy interested parties on deprecation
>   patches, so they can advise or object before they're a done deal.
>   The next commit will do that for libvirt.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-deprecated.texi | 234 ++
>  qemu-doc.texi| 235 +--
>  2 files changed, 235 insertions(+), 234 deletions(-)
>  create mode 100644 qemu-deprecated.texi

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

2018-07-16 Thread Daniel P . Berrangé
On Mon, Jul 16, 2018 at 09:32:26AM +0200, Markus Armbruster wrote:
> Libvirt developers would like to be copied on patches to qemu-doc
> appendix "Deprecated features".  Do them the favor.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 20eef3cb61..666e936812 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
>  S: Odd Fixes
>  F: docs/devel/build-system.txt
>  
> +Incompatible changes
> +R: libvir-list@redhat.com
> +F: qemu-deprecated.texi
> +
>  Build System
>  
>  GIT submodules

Reviewed-by: Daniel P. Berrangé 


Note, that libvir-list requires senders to be subscribed before mails are
delivered, otherwise they go into moderation queue. My normal approach. as
the person moderating, is to whitelist all human senders who get moderated,
so it is only a one-time pain point per person.

Thankyou spammers :-(

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

2018-07-16 Thread Daniel P . Berrangé
On Thu, Jul 12, 2018 at 05:47:00PM +0200, Thomas Huth wrote:
> On 12.07.2018 08:32, Markus Armbruster wrote:
> > Daniel P. Berrangé  writes:
> [...]
> >> For libvirt, I think whenever something is proposed for deprecation
> >> we could just CC libvir-list, or ask one of the libvirt people to
> >> confirm its not being used. If it is, then we should file BZ against
> >> libvirt.
> > 
> > Makes sense, but relying on developers getting their cc: right every
> > time is a setting us up for failure.
> > 
> > Our tool to help with getting cc: wrong less often is the MAINTAINERS
> > file.  Could one of the libvirt developers watch changes to qemu-doc
> > appendix "Deprecated features"?  Would putting the appendix in its own
> > .texi help with that?
> 
> Sound like a good idea to put the appendix in its own texi file. Then
> add an "R: libvir-list" entry for that file to MAINTAINERS and we should
> be fine (at least for the people who use the get_maintainers.pl script).

That's a neat idea and gets my vote.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 7/9] rpc: Alter virNetDaemonQuit processing

2018-07-16 Thread Daniel P . Berrangé
On Thu, Jul 12, 2018 at 03:34:00PM -0400, John Ferlan wrote:
> 
> 
> On 07/09/2018 03:11 AM, Peter Krempa wrote:
> > On Sat, Jul 07, 2018 at 08:11:05 -0400, John Ferlan wrote:
> >> When virNetDaemonQuit is called from libvirtd's shutdown
> >> handler (daemonShutdownHandler) we need to perform the quit
> >> in multiple steps. The first part is to "request" the quit
> >> and notify the NetServer's of the impending quit which causes
> >> the NetServers to inform their workers that a quit was requested.
> >>
> >> Still because we cannot guarantee a quit will happen or it's
> >> possible there's no workers pending, use a virNetDaemonQuitTimer
> >> to not only break the event loop but keep track of how long we're
> >> waiting and we've waited too long, force an ungraceful exit so
> >> that we don't hang waiting forever or cause some sort of SEGV
> >> because something is still pending and we Unref things.
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  src/libvirt_remote.syms|  1 +
> >>  src/remote/remote_daemon.c |  1 +
> >>  src/rpc/virnetdaemon.c | 68 +-
> >>  src/rpc/virnetdaemon.h |  4 +++
> >>  4 files changed, 73 insertions(+), 1 deletion(-)
> > 
> > [...]
> > 
> >> @@ -855,10 +904,27 @@ virNetDaemonRun(virNetDaemonPtr dmn)
> >>  virObjectLock(dmn);
> >>  
> >>  virHashForEach(dmn->servers, daemonServerProcessClients, NULL);
> >> +
> >> +/* HACK: Add a dummy timeout to break event loop */
> >> +if (dmn->quitRequested && quitTimer == -1)
> >> +quitTimer = virEventAddTimeout(500, virNetDaemonQuitTimer,
> >> +   &quitCount, NULL);
> >> +
> >> +if (dmn->quitRequested && daemonServerWorkersDone(dmn)) {
> >> +dmn->quit = true;
> >> +} else {
> >> +/* Firing every 1/2 second and quitTimeout in seconds, force
> >> + * an exit when there are still worker threads running and we
> >> + * have waited long enough */
> >> +if (quitCount > dmn->quitTimeout * 2)
> >> +_exit(EXIT_FAILURE);
> > 
> > If you have a legitimate long-running job which would finish eventually
> > and e.g. write an updated status XML this will break things. I'm not
> > persuaded that this is a systematic solution to some API getting stuck.
> > 
> > The commit message also does not help persuading me that this is a good
> > idea.
> > 
> > NACK
> > 
> 
> I understand the sentiment and this hasn't been a "top of the list" item
> for me to think about, but to a degree the purpose of re-posting the
> series was to try to come to a consensus over some solution. A naked
> NACK almost makes it appear as if you would prefer to not do anything in
> this situation, letting nature takes it's course (so to speak).
> 
> Do you have any thoughts or suggestions related to what processing you
> believe is appropriate if someone issues a SIG{INT|QUIT|TERM} to a
> daemon (libvirtd|lockd|logd}? That is what to do if we reach the
> cleanup: label in main() of src/remote/remote_daemon.c and
> virNetDaemonClose() gets run? Right now IIRC it's either going to be a
> SEGV or possible long wait for some worker thread to complete. The
> latter is the don't apply this example to cause a wait in block stats
> fetch. Naturally the long term wait only matters up through the point
> while someone is patient before issuing a SIGKILL.
> 
> Do you favor waiting forever for some worker thread to finish? To some
> degree if some signal is sent to libvirtd, then I would think the
> expectation is to not wait for some indeterminate time. And most
> definitely trying to avoid some sort of defunct state resolvable by a
> host reboot. Knowing exactly what a worker thread is doing may help, but
> that'd take a bit (;-)) of code to trace the stack. Perhaps providing a
> list of client/workers still active or even some indication that we're
> waiting on some worker rather than no feedback? How much is "too much"?
> 
> Another option that was proposed, but didn't really gain any support was
> introduction of a stateShutdown in virStateDriver that would be run
> during libvirtd's cleanup: processing prior to virNetDaemonClose. That
> would be before virStateCleanup. For qemu, the priv->mon and priv->agent
> would be closed. That would seemingly generate an error and cause the
> worker to exit thus theoretically not needing any forced quit of the
> thread "if" the reason was some sort of hang as a result of
> monitor/agent processing. Doesn't mean there wouldn't be some other
> issue causing a hang. Refreshing my memory on this - there was some
> discussion or investigation related to using virStateStop, but I since
> that's gated by "WITH_DBUS" being defined, it wasn't clear if/how it
> could be used (daemonStop is only called/setup if daemonRunStateInit has
> set it up).

I think the key thing to bear in mind is what is the benefit of what we
are trying todo. ie why 

Re: [libvirt] [PATCH 0/9] extend virsh domstate to show additional information

2018-07-16 Thread Bjoern Walk
Martin Kletzander  [2018-07-13, 10:50AM +0200]:
> I'm not very particularly opinionated on this, but I think APIs should be
> machine-readable and CLI tools can convert that to human-readable format.
> You'll never know when a program will like to access that and having to tell
> anyone in the future that they need to parse a string is ugly IMHO.

Ok, I can see the appeal for this. I agree that parsing the string is
not optimal, it just didn't occur to me that we maybe want to have this
information in other places as well.

So then I would model the new API function like Jiri suggested:

int
virDomainGetStateParams(virDomainPtr domain,
int *state,
int *reason,
virTypedParameterPtr *params,
int *nparams,
unsigned int flags)

where each parameter in params holds a piece of the additional
information, i.e VIR_DOMAIN_STATE_PARAMS_REASON_PSW_ADDR and so on on
s390x.

However, this raises the question how this would be handled on the
client, who now receives an unknown set of parameters. How would for
example virsh go on and reconstruct the info string? Just iterate of all
returned parameters? Or should it introspect the parameters and perform
an explicit formatting?

> Also from the monitor you can get that information only if there is any QEMU
> running.  I presume the state you are returning is saved somewhere along with
> the reason so that it can be provided later.

Yeah, in my QEMU-centric view I just assumed that this was an option.
For where this information is saved, if I understand you correctly, I am
not sure if I want to clutter this into the domain object where the
state and reasons (and, in this patch series, the info) is saved. I try
to find a way to retrieve this information from the hypervisor on the
fly.

I don't expect to much changes for the backend, so a general review of
the code if I am heading in the right direction.would be appreciated.

> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


-- 
IBM Systems
Linux on Z & Virtualization Development

IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819

Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

2018-07-16 Thread Cornelia Huck
On Mon, 16 Jul 2018 09:54:12 +0200
Thomas Huth  wrote:

> On 16.07.2018 09:32, Markus Armbruster wrote:
> > Libvirt developers would like to be copied on patches to qemu-doc
> > appendix "Deprecated features".  Do them the favor.
> > 
> > Signed-off-by: Markus Armbruster 
> > ---
> >  MAINTAINERS | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 20eef3cb61..666e936812 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
> >  S: Odd Fixes
> >  F: docs/devel/build-system.txt
> >  
> > +Incompatible changes
> > +R: libvir-list@redhat.com
> > +F: qemu-deprecated.texi  
> 
> Should we have a maintainer for the file, too? (I guess not, because
> deprecation patches should go through the specific subsystems...)

I don't think adding a maintainer makes sense for this file.

> And what about a "S:" line?

I don't think that makes too much sense, either.

If anything, qemu-deprecated.texi should be in a category 'maintained
by everybody', i.e. qemu-devel. Just like qemu-doc.texi, which does not
have an entry in MAINTAINERS at all.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

2018-07-16 Thread Cornelia Huck
On Mon, 16 Jul 2018 09:32:26 +0200
Markus Armbruster  wrote:

> Libvirt developers would like to be copied on patches to qemu-doc
> appendix "Deprecated features".  Do them the favor.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Cornelia Huck 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

2018-07-16 Thread Thomas Huth
On 16.07.2018 09:32, Markus Armbruster wrote:
> Libvirt developers would like to be copied on patches to qemu-doc
> appendix "Deprecated features".  Do them the favor.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 20eef3cb61..666e936812 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
>  S: Odd Fixes
>  F: docs/devel/build-system.txt
>  
> +Incompatible changes
> +R: libvir-list@redhat.com
> +F: qemu-deprecated.texi

Should we have a maintainer for the file, too? (I guess not, because
deprecation patches should go through the specific subsystems...)
And what about a "S:" line?

Anyway:

Reviewed-by: Thomas Huth 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

2018-07-16 Thread Cornelia Huck
On Mon, 16 Jul 2018 09:32:25 +0200
Markus Armbruster  wrote:

> Consumers of QEMU need to track feature deprecation.  Keeping
> deprecation documentation in its own file helps in two small ways:
> 
> * You can track changes the easy and obvious way, with git-log.
>   Before, you had to resort to more complex gittery like "git-log
>   --oneline -L '/@node Deprecated features/,/@node Supported build
>   platforms/:qemu-doc.texi'"
> 
> * It lets us use MAINTAINERS to copy interested parties on deprecation
>   patches, so they can advise or object before they're a done deal.
>   The next commit will do that for libvirt.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-deprecated.texi | 234 ++
>  qemu-doc.texi| 235 +--
>  2 files changed, 235 insertions(+), 234 deletions(-)
>  create mode 100644 qemu-deprecated.texi

Reviewed-by: Cornelia Huck 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

2018-07-16 Thread Thomas Huth
On 16.07.2018 09:32, Markus Armbruster wrote:
> Consumers of QEMU need to track feature deprecation.  Keeping
> deprecation documentation in its own file helps in two small ways:
> 
> * You can track changes the easy and obvious way, with git-log.
>   Before, you had to resort to more complex gittery like "git-log
>   --oneline -L '/@node Deprecated features/,/@node Supported build
>   platforms/:qemu-doc.texi'"
> 
> * It lets us use MAINTAINERS to copy interested parties on deprecation
>   patches, so they can advise or object before they're a done deal.
>   The next commit will do that for libvirt.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-deprecated.texi | 234 ++
>  qemu-doc.texi| 235 +--
>  2 files changed, 235 insertions(+), 234 deletions(-)
>  create mode 100644 qemu-deprecated.texi

Reviewed-by: Thomas Huth 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

2018-07-16 Thread Markus Armbruster
Consumers of QEMU need to track feature deprecation.  Keeping
deprecation documentation in its own file helps in two small ways:

* You can track changes the easy and obvious way, with git-log.
  Before, you had to resort to more complex gittery like "git-log
  --oneline -L '/@node Deprecated features/,/@node Supported build
  platforms/:qemu-doc.texi'"

* It lets us use MAINTAINERS to copy interested parties on deprecation
  patches, so they can advise or object before they're a done deal.
  The next commit will do that for libvirt.

Signed-off-by: Markus Armbruster 
---
 qemu-deprecated.texi | 234 ++
 qemu-doc.texi| 235 +--
 2 files changed, 235 insertions(+), 234 deletions(-)
 create mode 100644 qemu-deprecated.texi

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
new file mode 100644
index 00..9920a85adc
--- /dev/null
+++ b/qemu-deprecated.texi
@@ -0,0 +1,234 @@
+@node Deprecated features
+@appendix Deprecated features
+
+In general features are intended to be supported indefinitely once
+introduced into QEMU. In the event that a feature needs to be removed,
+it will be listed in this appendix. The feature will remain functional
+for 2 releases prior to actual removal. Deprecated features may also
+generate warnings on the console when QEMU starts up, or if activated
+via a monitor command, however, this is not a mandatory requirement.
+
+Prior to the 2.10.0 release there was no official policy on how
+long features would be deprecated prior to their removal, nor
+any documented list of which features were deprecated. Thus
+any features deprecated prior to 2.10.0 will be treated as if
+they were first deprecated in the 2.10.0 release.
+
+What follows is a list of all features currently marked as
+deprecated.
+
+@section Build options
+
+@subsection GTK 2.x
+
+Previously QEMU has supported building against both GTK 2.x
+and 3.x series APIs. Support for the GTK 2.x builds will be
+discontinued, so maintainers should switch to using GTK 3.x,
+which is the default.
+
+@subsection SDL 1.2
+
+Previously QEMU has supported building against both SDL 1.2
+and 2.0 series APIs. Support for the SDL 1.2 builds will be
+discontinued, so maintainers should switch to using SDL 2.0,
+which is the default.
+
+@section System emulator command line arguments
+
+@subsection -no-kvm (since 1.3.0)
+
+The ``-no-kvm'' argument is now a synonym for setting
+``-machine accel=tcg''.
+
+@subsection -vnc tls (since 2.5.0)
+
+The ``-vnc tls'' argument is now a synonym for setting
+``-object tls-creds-anon,id=tls0'' combined with
+``-vnc tls-creds=tls0'
+
+@subsection -vnc x509 (since 2.5.0)
+
+The ``-vnc x509=/path/to/certs'' argument is now a
+synonym for setting
+``-object tls-creds-x509,dir=/path/to/certs,id=tls0,verify-peer=no''
+combined with ``-vnc tls-creds=tls0'
+
+@subsection -vnc x509verify (since 2.5.0)
+
+The ``-vnc x509verify=/path/to/certs'' argument is now a
+synonym for setting
+``-object tls-creds-x509,dir=/path/to/certs,id=tls0,verify-peer=yes''
+combined with ``-vnc tls-creds=tls0'
+
+@subsection -tftp (since 2.6.0)
+
+The ``-tftp /some/dir'' argument is replaced by either
+``-netdev user,id=x,tftp=/some/dir '' (for pluggable NICs, accompanied
+with ``-device ...,netdev=x''), or ``-nic user,tftp=/some/dir''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -bootp (since 2.6.0)
+
+The ``-bootp /some/file'' argument is replaced by either
+``-netdev user,id=x,bootp=/some/file '' (for pluggable NICs, accompanied
+with ``-device ...,netdev=x''), or ``-nic user,bootp=/some/file''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -redir (since 2.6.0)
+
+The ``-redir [tcp|udp]:hostport:[guestaddr]:guestport'' argument is
+replaced by either
+``-netdev 
user,id=x,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport''
+(for pluggable NICs, accompanied with ``-device ...,netdev=x'') or
+``-nic user,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -smb (since 2.6.0)
+
+The ``-smb /some/dir'' argument is replaced by either
+``-netdev user,id=x,smb=/some/dir '' (for pluggable NICs, accompanied
+with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0)
+
+The drive geometry arguments are replaced by the the geometry arguments
+that can be specified with the ``-device'' parameter.
+
+@subsection -drive serial=... (since 2.10.0)
+
+The drive serial argument is replaced by the the serial argument
+that can be specified with the ``-device'' parameter.
+
+@subsection -drive addr=... (since 2.10.0)
+
+The drive addr argument is replaced by th

[libvirt] [PATCH 0/2] Make it easier to track feature deprecation

2018-07-16 Thread Markus Armbruster
Markus Armbruster (2):
  qemu-doc: Move appendix "Deprecated features" to its own file
  MAINTAINERS: New section "Incompatible changes", copy libvir-list

 MAINTAINERS  |   4 +
 qemu-deprecated.texi | 234 ++
 qemu-doc.texi| 235 +--
 3 files changed, 239 insertions(+), 234 deletions(-)
 create mode 100644 qemu-deprecated.texi

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

2018-07-16 Thread Markus Armbruster
Libvirt developers would like to be copied on patches to qemu-doc
appendix "Deprecated features".  Do them the favor.

Signed-off-by: Markus Armbruster 
---
 MAINTAINERS | 4 
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 20eef3cb61..666e936812 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
 S: Odd Fixes
 F: docs/devel/build-system.txt
 
+Incompatible changes
+R: libvir-list@redhat.com
+F: qemu-deprecated.texi
+
 Build System
 
 GIT submodules
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: hotplug: report error when changing rom enabled attr for net iface

2018-07-16 Thread Andrea Bolognani
On Fri, 2018-07-13 at 16:07 +0200, Ján Tomko wrote:
> On Fri, Jul 13, 2018 at 03:57:04PM +0200, Katerina Koukiou wrote:
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1599513
> 
> The 'Resolves: ' prefix is still pointless, but Andrea might try to disagree

Nah, I'm good :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list