[python-libvirt] RFC:PEP 484 type annotation - how to continue

2020-10-05 Thread Philipp Hahn
Hello,

currently my Merge Quests
 is stuck
because there are some open technical discussions:

1. The Python binding consists of two layers:
   - libvirtmod* is the low-level C-like API, which mostly is just a
thin wrapper around the C-library. It is generated from the API.xml files
  - The high-level libvirt* modules, providing an object oriented API,
which is also generated from the API.xml files.
  In both cases there are many exceptions defines via
libvirt*-override-api.xml or coded in generator.py:
  - skip_impl: Generate Python code, but skip C implementation
  - custom_function: Generate neither C nor Python code - hand crafted code
  - skip_function: Generate neither C nor Python code - not used at all
  - function_skip_python_impl: Generate C code, but skip python impl

Previously the functions listed in "skip_impl" were also duplicated in
the "libvirt-*override-api.xml" files, which is somehow hard to
maintain. With d19699e48ce9c17481f94b4dc0715e18e05d5adb I removed all
those names from "skip_impl", which are also listed in the overrides and
added code to add them back when "generator.py" is invoked.
For this to work I had to change several "files" attributes to start
with "python", which looks okay to me, but still would like to get verified.

2. The low-level library works mostly with bytes, but the high-level
Python API uses nested Lists, Tuples and Dicts. To get most out of the
Python static type annotations the input and output parameters should be
modled in as much detail as possible.
This requires to store those types somewhere. My current
46592e28124858ed863bceba65c3dafa2bbb02cd adds some often used types to
generator.py and then references them from the overrides.xml:

> +'PyListSecret': ('', 'List["virSecret"]', '', ''),
> +'PyDictAny': ('', 'Dict[str, Any]', '', ''),

> +  

This still is very painful as there are many "used-once" types, which
still must be declared and used in two places.
So far the "return" type was used for the "C" type and currently I
mis-use it for referencing it for my Python types. This doesn't look
right, so I would prefer adding a new attribute which can be used to
override the Python type.
This also would probably nicely fit with functions like
"virDomainOpenConsole", when "dev_name" is optional and can be "None",
which the current type hint requires a "string".

Is extending the libvirt*-override-api.xml files okay or must this be
coordinated with the other parts of the project?


Thank you for your time
Philipp Hahn
-- 
Philipp Hahn
Open Source Software Engineer

Univention GmbH
be open.
Mary-Somerville-Str. 1
D-28359 Bremen

📞 +49-421-22232-57
🖶 +49-421-22232-99

✉️ h...@univention.de
🌐 https://www.univention.de/

Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876




Re: [PATCH 1/1] Optional free-page-reporting can be enabled/disabled for memballon device of model 'virtio'.

2020-10-05 Thread Peter Krempa
On Fri, Oct 02, 2020 at 11:59:43 -0400, Nico Pache wrote:
> xml:
> 
>   

according to what you state in the cover-letter this is a
'virtio-balloon-pci' feature. We usually put stuff which depends on a
specific model of the device into the  subelement of the device
element.

> 
> 
> qemu:
> qemu -device virtio-balloon-pci,...,free-page-reporting=on
> 
> This kernel feature allows for more stable and resource friendly systems.
> 
> Signed-off-by: Nico Pache 
> ---

We require that changes are separated to smaller chunks according to the
following logical boundaries:

>  docs/formatdomain.rst |  6 ++
>  docs/schemas/domaincommon.rng |  5 +
>  src/conf/domain_conf.c| 21 +++
>  src/conf/domain_conf.h|  1 +

Docs/RNG schema and parser/formatter needs to be separate

>  src/qemu/qemu_capabilities.c  |  4 +++-
>  src/qemu/qemu_capabilities.h  |  1 +

>  .../caps_5.1.0.x86_64.xml |  1 +
>  .../caps_5.2.0.x86_64.xml |  1 +

qemu caps changes should be separate

>  src/qemu/qemu_command.c   |  5 +
>  src/qemu/qemu_validate.c  |  7 +++

And qemu itself should be separate.

In case you are going to add a NEWS.rst entry for this addition, any
change to NEWS.rst also must be in a separate commit.

>  10 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index f3cf9e1fb3..f4c7765f5f 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -6755,6 +6755,12 @@ Example: manually added device with static PCI slot 2 
> requested
> release some memory at the last moment before a guest's process get 
> killed by
> Out of Memory killer. :since:`Since 1.3.1, QEMU and KVM only`
>  
> +``free-page-reporting``
> +   The optional ``free-page-reporting`` attribute allows to enable/disable
> +   ("on"/"off", respectively) the ability of the QEMU virtio memory balloon 
> to
> +   return unused pages back to the hypervisor to be used by other guests or
> +   processes. :since:`Since 5.1, QEMU and KVM only`

There's a discrepancy between the feature name and the description. The
feature name seems to suggest that you can query the amount of free
pages somewhere, whereas the description states that it's actually
returning the pages to the host.

If the latter is true the feature should be named accordingly to prevent
confusion.

[...]

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 5dcfcd574d..88a2c8aacb 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -600,7 +600,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
>  
>/* 380 */
>"usb-host.hostdevice",
> -);
> +  "virtio-balloon-pci.free-page-reporting",
> +);

Plase don't change alignment of this brace.

>  typedef struct _virQEMUCapsMachineType virQEMUCapsMachineType;

[...]

> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index a212605579..cfeb1e67c4 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -3926,6 +3926,13 @@ qemuValidateDomainDeviceDefMemballoon(const 
> virDomainMemballoonDef *memballoon,
>  return -1;
>  }
>  
> +if (memballoon->free_page_reporting != VIR_TRISTATE_SWITCH_ABSENT &&
> + !virQEMUCapsGet(qemuCaps, 
> QEMU_CAPS_VIRTIO_BALLOON_FREE_PAGE_REPORTING)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +_("free-page-reporting is not supported by this QEMU 
> binary"));
> + return -1;
> + }

Is it worth forbidding disabling the feature if it isn't supported?

> +
>  if (qemuValidateDomainVirtioOptions(memballoon->virtio, qemuCaps) < 0)
>  return -1;

The patch is also missing test XML addition for the qemuxml2argvtest and
qemuxml2xmltest.



Re: [PATCH] qemu: Taint cpu host-passthrough only after migration

2020-10-05 Thread Daniel P . Berrangé
On Sun, Oct 04, 2020 at 03:10:52PM -0400, Cole Robinson wrote:
> From a discussion last year[1], Dan recommended libvirt drop the tain
> flag for cpu host-passthrough, unless the VM has been migrated.
> 
> This repurposes the existing host-cpu taint flag to do just that.
> 
> [1]: 
> https://www.redhat.com/archives/virt-tools-list/2019-February/msg00041.html
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1673098
> 
> Signed-off-by: Cole Robinson 
> ---
>  src/conf/domain_conf.h  | 2 +-
>  src/qemu/qemu_domain.c  | 7 +--
>  src/qemu/qemu_domain.h  | 3 ++-
>  src/qemu/qemu_process.c | 2 +-
>  4 files changed, 9 insertions(+), 5 deletions(-)

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



Re: [libvirt PATCH 8/8] build: lower maximum frame size to 1792

2020-10-05 Thread Daniel P . Berrangé
On Mon, Oct 05, 2020 at 12:21:45AM +0200, Ján Tomko wrote:
> This number is the closest multiple of 1

  ^^^ Huh ?

> above the largest frame value reported by clang
> in the current codebase.
> 
> Signed-off-by: Ján Tomko 
> ---
>  meson.build | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index a5ce8e17a8..8bef701f67 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -419,10 +419,9 @@ cc_flags += [
># but need to rewrite various areas of code first
>'-Wno-format-truncation',
>  
> -  # This should be < 256 really. Currently we're down to 4096,
> -  # but using 1024 bytes sized buffers (mostly for virStrerror)
> -  # stops us from going down further
> -  '-Wframe-larger-than=4096',
> +  # This should be < 256 really.

I think we can drop this as I doubt it is worth trying to achieve.
We've not knowingly had stack overflow in livirt in history.

> +  # Using 1024 bytes sized buffers stops us from going down further
> +  '-Wframe-larger-than=1792',


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



Re: [libvirt PATCH 8/8] build: lower maximum frame size to 1792

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Daniel P. Berrangé wrote:

On Mon, Oct 05, 2020 at 12:21:45AM +0200, Ján Tomko wrote:

This number is the closest multiple of 1


 ^^^ Huh ?



0x100 for short


above the largest frame value reported by clang
in the current codebase.

Signed-off-by: Ján Tomko 
---
 meson.build | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index a5ce8e17a8..8bef701f67 100644
--- a/meson.build
+++ b/meson.build
@@ -419,10 +419,9 @@ cc_flags += [
   # but need to rewrite various areas of code first
   '-Wno-format-truncation',

-  # This should be < 256 really. Currently we're down to 4096,
-  # but using 1024 bytes sized buffers (mostly for virStrerror)
-  # stops us from going down further
-  '-Wframe-larger-than=4096',
+  # This should be < 256 really.


I think we can drop this as I doubt it is worth trying to achieve.
We've not knowingly had stack overflow in livirt in history.


Yeah, 256 is impossible to achieve. Especially since clang seems
more conservative with the numbers it prints.

I merely deleted the parts of the comment with outdated information.

Jano




+  # Using 1024 bytes sized buffers stops us from going down further
+  '-Wframe-larger-than=1792',



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


signature.asc
Description: PGP signature


Re: restrictions for virtiofs (related to commit: 88957116c9 for libvirt 6.9.0)

2020-10-05 Thread Michal Privoznik

On 10/3/20 1:15 AM, Masayoshi Mizuma wrote:

On Fri, Oct 02, 2020 at 11:31:32AM -0400, Masayoshi Mizuma wrote:

Hello Jan, and Michal,

commit: 88957116c9 ("qemu: Use memory-backend-* for regular guest memory") gets
the system memory sharable without numa config.
The qemu options with the patch will be like as:

   -machine 
pc-q35-5.2,accel=kvm,usb=off,vmport=off,smm=on,dump-guest-core=off,memory-backend=pc.ram
 \
   -object 
memory-backend-file,id=pc.ram,mem-path=/var/lib/libvirt/qemu/ram/2-Test/pc.ram,share=yes,size=17179869184
 \

So, we can remove the numa restriction of virtiofs, right?
The patch to remove that is the bottom of this email.

And, 88957116c9 seems to introduce another restriction which we
cannot create numa nodes on the machine. I got the following
message when I set the numa config and started the VM:

2020-10-02T00:31:46.780374Z qemu-system-x86_64: '-machine memory-backend' 
and '-numa memdev' properties are mutually exclusive


It seems that this isn't a restriction for virtiofs. It's a bug introduced
by commit: 88957116c9.  element doesn't work regardless of virtiofs 
config...


Do you have domain XML that is failing?

Michal



Re: [PATCH] rpm: Simplify qemu+kvm conditionals and eliminate duplication

2020-10-05 Thread Pavel Hrdina
On Thu, Sep 24, 2020 at 04:04:38PM -0400, Neal Gompa wrote:
> The conditionals for enabling qemu+kvm were unnecessarily complex.
> In practice, there were far fewer cases where the functionality would
> be disabled than what the conditional logic expressed, and this change
> simplifies it to the realistic set of options.
> 
> Signed-off-by: Neal Gompa 
> ---
>  libvirt.spec.in | 49 +++--
>  1 file changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index c4a7c30737..6940066de9 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -20,31 +20,12 @@
>  
>  %define with_qemu_tcg  %{with_qemu}
>  
> -%define qemu_kvm_arches %{ix86} x86_64
> -
> -%if 0%{?fedora}
> -%define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64
> -%endif
> -
> -%if 0%{?rhel}
> -%define with_qemu_tcg 0
> -%define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
> -%endif
> -
>  # On RHEL 7 and older macro _vpath_builddir is not defined.
>  %if 0%{?rhel} && 0%{?rhel} <= 7
>  %define _vpath_builddir %{_target_platform}
>  %endif
>  
> -%ifarch %{qemu_kvm_arches}
> -%define with_qemu_kvm  %{with_qemu}
> -%else
> -%define with_qemu_kvm  0
> -%endif
> -
> -%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
> -%define with_qemu 0
> -%endif
> +%define with_qemu_kvm  %{with_qemu}

I would move this line above the rhel-7 _vpath_builddir workaround to
have it next to with_qemu_tcg.

>  # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
>  %define with_openvz0%{!?_without_openvz:1}
> @@ -61,12 +42,6 @@
>  %endif
>  
>  %define with_storage_gluster 0%{!?_without_storage_gluster:1}
> -%ifnarch %{qemu_kvm_arches}
> -# gluster is only built where qemu driver is enabled on RHEL 8
> -%if 0%{?rhel} >= 8
> -%define with_storage_gluster 0
> -%endif
> -%endif
>  
>  %define with_numactl  0%{!?_without_numactl:1}
>  
> @@ -97,6 +72,11 @@
>  
>  # Finally set the OS / architecture specific special cases
>  
> +# KVM is available on most architectures
> +%ifnarch %{ix86} x86_64 %{power64} s390x %{arm} aarch64
> +%define with_qemu_kvm 0
> +%endif
> +
>  # Xen is available only on i386 x86_64 ia64
>  %ifnarch %{ix86} x86_64 ia64
>  %define with_libxl 0
> @@ -122,6 +102,23 @@
>  %define with_storage_rbd 0
>  %endif
>  
> +# RHEL does not ship qemu-tcg
> +%if 0%{?rhel}
> +%define with_qemu_tcg 0
> +%endif
> +
> +# In the event that qemu-tcg and qemu-kvm are unavailable, don't ship qemu
> +%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
> +%define with_qemu 0
> +%endif
> +
> +%if ! %{with_qemu_kvm}
> +# gluster is only built where qemu driver is enabled on RHEL 8
> +%if 0%{?rhel} >= 8
> +%define with_storage_gluster 0
> +%endif
> +%endif
> +
>  # RHEL doesn't ship OpenVZ, VBox, PowerHypervisor,
>  # VMware, libxenlight (Xen 4.1 and newer),
>  # or HyperV.

There is one more place with qemu_kvm_arches:

%if 0%{?rhel}
%ifarch %{qemu_kvm_arches}
%define with_sanlock 0%{!?_without_sanlock:1}
%endif
%endif

Otherwise looks good.

Pavel


signature.asc
Description: PGP signature


Re: [PATCH 1/1] Optional free-page-reporting can be enabled/disabled for memballon device of model 'virtio'.

2020-10-05 Thread David Hildenbrand
On 05.10.20 10:06, Peter Krempa wrote:
> On Fri, Oct 02, 2020 at 11:59:43 -0400, Nico Pache wrote:
>> xml:
>> 
>>   
> 
> according to what you state in the cover-letter this is a
> 'virtio-balloon-pci' feature. We usually put stuff which depends on a
> specific model of the device into the  subelement of the device
> element.

IIRC, this should work for virtio-balloon-*, including virtio-balloon-ccw.


-- 
Thanks,

David / dhildenb



Re: [PATCH 1/1] Optional free-page-reporting can be enabled/disabled for memballon device of model 'virtio'.

2020-10-05 Thread Pavel Hrdina
On Mon, Oct 05, 2020 at 10:06:20AM +0200, Peter Krempa wrote:
> On Fri, Oct 02, 2020 at 11:59:43 -0400, Nico Pache wrote:

[...]

> > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> > index a212605579..cfeb1e67c4 100644
> > --- a/src/qemu/qemu_validate.c
> > +++ b/src/qemu/qemu_validate.c
> > @@ -3926,6 +3926,13 @@ qemuValidateDomainDeviceDefMemballoon(const 
> > virDomainMemballoonDef *memballoon,
> >  return -1;
> >  }
> >  
> > +if (memballoon->free_page_reporting != VIR_TRISTATE_SWITCH_ABSENT &&
> > + !virQEMUCapsGet(qemuCaps, 
> > QEMU_CAPS_VIRTIO_BALLOON_FREE_PAGE_REPORTING)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +_("free-page-reporting is not supported by this 
> > QEMU binary"));
> > + return -1;
> > + }
> 
> Is it worth forbidding disabling the feature if it isn't supported?

I would say that explicit usage of a feature that is not available in
QEMU should result in error.

Pavel


signature.asc
Description: PGP signature


Re: [PATCH 1/1] Optional free-page-reporting can be enabled/disabled for memballon device of model 'virtio'.

2020-10-05 Thread Peter Krempa
On Mon, Oct 05, 2020 at 11:11:54 +0200, David Hildenbrand wrote:
> On 05.10.20 10:06, Peter Krempa wrote:
> > On Fri, Oct 02, 2020 at 11:59:43 -0400, Nico Pache wrote:
> >> xml:
> >> 
> >>   
> > 
> > according to what you state in the cover-letter this is a
> > 'virtio-balloon-pci' feature. We usually put stuff which depends on a
> > specific model of the device into the  subelement of the device
> > element.
> 
> IIRC, this should work for virtio-balloon-*, including virtio-balloon-ccw.

In that case the qemu capability detection should be added also for the
ccw version and capability name needs to be modified to not include pci.



Re: [PATCH 03/15] virBitmapToString|virBitmapNewString: Clarify semantics of the 'string'

2020-10-05 Thread Peter Krempa
On Fri, Oct 02, 2020 at 17:44:13 +0200, Ján Tomko wrote:
> On a Friday in 2020, Peter Krempa wrote:
> > On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
> > > On a Friday in 2020, Peter Krempa wrote:
> > > > Clarify which bit is considered most significant in the bitmap and
> > > > resulting string. Also be explicit that it's a hex string.
> > > >
> > > > Signed-off-by: Peter Krempa 
> > > > ---
> > > > src/util/virbitmap.c | 13 +
> > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> > > > index ad5213f216..fcb8e1101a 100644
> > > > --- a/src/util/virbitmap.c
> > > > +++ b/src/util/virbitmap.c
> > > > @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap,
> > > >  * virBitmapToString:
> > > >  * @bitmap: Pointer to bitmap
> > > >  *
> > > > - * Convert @bitmap to printable string.
> > > > + * Convert @bitmap to printable hexadecimal string representation. 
> > > > Note that bit
> > > > + * with highest position/index in @bitmap are considered as most 
> > > > significant bit
> > > > + * in the output string.
> > > 
> > > the bits ... are considered
> > >  or
> > > the bit ... is considered
> > 
> > oops, I've rewrote it halfway through ...
> > 
> > > 
> > > would mentioning that it is printed at the leftmost position be clearer?
> > 
> > Well, the thing is that the leftmost digit in the output string
> > represents more than one bit since it's hex. I thought about some
> > wordign but couldn't come up with anything more appropriate.
> > 
> > We could do: 'is considered as the most significant bit of the number
> > represented by the output string', or just 'most significant bit of the
> > output number". That way the reader knows it's a number and the
> > semantics of the bit are then implicit.
> 
> Either of those LGTM

I'll go with:

diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index ad5213f216..ed28427736 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap,
  * virBitmapToString:
  * @bitmap: Pointer to bitmap
  *
- * Convert @bitmap to printable string.
+ * Convert @bitmap to a number where the bit with highest position/index in
+ * @bitmap represents the most significant bit and return the number in form
+ * of a hexadecimal string.
  *
  * Returns pointer to the string or NULL on error.
  */
@@ -1117,10 +1119,14 @@ virBitmapCountBits(virBitmapPtr bitmap)
  * virBitmapNewString:
  * @string: the string to be converted to a bitmap
  *
- * Allocate a bitmap from a string of hexadecimal data.
+ * Allocate a bitmap and populate it from @string representing a number in
+ * hexadecimal format. Note that the most significant bit of the number
+ * represented by @string will correspond to the highest index/position in the
+ * bitmap. The size of the returned bitmap corresponds to 4 * the length of
+ * @string.
  *
- * Returns a pointer to the allocated bitmap or NULL if
- * memory cannot be allocated.
+ * Returns a pointer to the allocated bitmap or NULL and reports an error if
+ * @string can't be converted.
  */
 virBitmapPtr
 virBitmapNewString(const char *string)



Re: [PATCH 03/15] virBitmapToString|virBitmapNewString: Clarify semantics of the 'string'

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

On Fri, Oct 02, 2020 at 17:44:13 +0200, Ján Tomko wrote:

On a Friday in 2020, Peter Krempa wrote:
> On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
> > On a Friday in 2020, Peter Krempa wrote:
> > > Clarify which bit is considered most significant in the bitmap and
> > > resulting string. Also be explicit that it's a hex string.
> > >
> > > Signed-off-by: Peter Krempa 
> > > ---
> > > src/util/virbitmap.c | 13 +
> > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> > > index ad5213f216..fcb8e1101a 100644
> > > --- a/src/util/virbitmap.c
> > > +++ b/src/util/virbitmap.c
> > > @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap,
> > >  * virBitmapToString:
> > >  * @bitmap: Pointer to bitmap
> > >  *
> > > - * Convert @bitmap to printable string.
> > > + * Convert @bitmap to printable hexadecimal string representation. Note 
that bit
> > > + * with highest position/index in @bitmap are considered as most 
significant bit
> > > + * in the output string.
> >
> > the bits ... are considered
> >  or
> > the bit ... is considered
>
> oops, I've rewrote it halfway through ...
>
> >
> > would mentioning that it is printed at the leftmost position be clearer?
>
> Well, the thing is that the leftmost digit in the output string
> represents more than one bit since it's hex. I thought about some
> wordign but couldn't come up with anything more appropriate.
>
> We could do: 'is considered as the most significant bit of the number
> represented by the output string', or just 'most significant bit of the
> output number". That way the reader knows it's a number and the
> semantics of the bit are then implicit.

Either of those LGTM


I'll go with:

diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index ad5213f216..ed28427736 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap,
 * virBitmapToString:
 * @bitmap: Pointer to bitmap
 *
- * Convert @bitmap to printable string.
+ * Convert @bitmap to a number where the bit with highest position/index in
+ * @bitmap represents the most significant bit and return the number in form
+ * of a hexadecimal string.
 *
 * Returns pointer to the string or NULL on error.
 */
@@ -1117,10 +1119,14 @@ virBitmapCountBits(virBitmapPtr bitmap)
 * virBitmapNewString:
 * @string: the string to be converted to a bitmap
 *
- * Allocate a bitmap from a string of hexadecimal data.
+ * Allocate a bitmap and populate it from @string representing a number in
+ * hexadecimal format. Note that the most significant bit of the number
+ * represented by @string will correspond to the highest index/position in the
+ * bitmap. The size of the returned bitmap corresponds to 4 * the length of
+ * @string.
 *
- * Returns a pointer to the allocated bitmap or NULL if
- * memory cannot be allocated.
+ * Returns a pointer to the allocated bitmap or NULL and reports an error if
+ * @string can't be converted.
 */
virBitmapPtr
virBitmapNewString(const char *string)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [libvirt PATCH] API: discourage usage of non-ListAll APIs

2020-10-05 Thread Andrea Bolognani
On Mon, 2020-10-05 at 00:35 +0200, Ján Tomko wrote:
> +++ b/src/libvirt-domain-snapshot.c
> @@ -378,8 +378,10 @@ virDomainSnapshotNum(virDomainPtr domain, unsigned int 
> flags)
>   * snapshots were listed if the return is less than @nameslen.  Likewise,
>   * you should be prepared for virDomainSnapshotLookupByName() to fail when
>   * converting a name from this call into a snapshot object, if another
> - * connection deletes the snapshot in the meantime.  For more control over
> - * the results, see virDomainListAllSnapshots().
> + * connection deletes the snapshot in the meantime.
> + *
> + * The use of this function in new code is discouraged. Instead, use
> + * virDomainListAllSnapshots().

I would tweak this slightly to

  The use of this function is discouraged, use Replacement() instead.

My argument for removing the "in new code" bit is that use of the old
APIs is a code smell, and even if everything appears to work fine
there are probably subtle bugs hiding right underneath the surface,
and so migrating to the newer APIs should be considered even for
existing libvirt-using projects.

Also I believe you missed virDomainSnapshotListChildrenNames().

> +++ b/src/libvirt-nodedev.c
> @@ -151,7 +151,8 @@ virConnectListAllNodeDevices(virConnectPtr conn,
>   *
>   * Collect the list of node devices, and store their names in @names
>   *
> - * For more control over the results, see virConnectListAllNodeDevices().
> + * The use of this function in new code is discouraged. Instead, use
> + * virConnectListAllNodeDevices().

Unrelated to your patch, but we should probably introduce a
virNodeDeviceListAllCaps() API here.

> +++ b/src/libvirt-nwfilter.c
> @@ -117,6 +117,9 @@ virConnectListAllNWFilters(virConnectPtr conn,
>   *
>   * Collect the list of network filters, and store their names in @names
>   *
> + * The use of this function in new code is discouraged. Instead, use
> + * virConnectListALLNWFilters().

s/ALL/All/


Should we consider adding the same notice to

  * virDomainSnapshotNum()
  * virDomainSnapshotNumChildren()
  * virConnectNumOfDefinedDomains()
  * virConnectNumOfDomains()
  * virConnectNumOfInterfaces()
  * virConnectNumOfDefinedInterfaces()
  * virConnectNumOfNetworks()
  * virConnectNumOfDefinedNetworks()
  * virNodeNumOfDevices()
  * virConnectNumOfNWFilters()
  * virConnectNumOfSecrets()
  * virConnectNumOfStoragePools()
  * virConnectNumOfDefinedStoragePools()
  * virStoragePoolNumOfVolumes()

or is there a reasonable situation in which you might want to call
those APIs and then pass the result to something other than the
various List() APIs?

Overally looks good! Thank you for taking the time to do this :)

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [libvirt PATCH 4/4] vbox: use g_new0 instead of VIR_ALLOC

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 12:22:26AM +0200, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
...

>  
>  for (i = 0; i < sharedFolders.count; i++) {
>  ISharedFolder *sharedFolder = sharedFolders.items[i];
> @@ -3798,10 +3789,8 @@ vboxDumpAudio(virDomainDefPtr def, vboxDriverPtr data 
> G_GNUC_UNUSED,
>  PRUint32 audioController = AudioControllerType_AC97;
>  
>  def->nsounds = 1;
> -if (VIR_ALLOC_N(def->sounds, def->nsounds) < 0)
> -return;
> -if (VIR_ALLOC(def->sounds[0]) < 0)
> -return;
> +def->sounds = g_new0(virDomainSoundDefPtr, def->nsounds);

just use 1 for the counter and drop the nsounds attribute initialization
completely.

Reviewed-by: Erik Skultety 



Re: [libvirt PATCH 0/4] vbox: use g_new0 (glib chronicles)

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 12:22:22AM +0200, Ján Tomko wrote:
> Ján Tomko (4):
>   vbox: use g_new0 in vboxDumpAudio
>   vbox: invert condition in vboxSnapshotGetReadOnlyDisks
>   vbox: refactor vboxNetworkGetXMLDesc a bit
>   vbox: use g_new0 instead of VIR_ALLOC
> 
>  src/vbox/vbox_common.c| 119 --
>  src/vbox/vbox_network.c   | 107 ++
>  src/vbox/vbox_snapshot_conf.c |  48 ++
>  3 files changed, 110 insertions(+), 164 deletions(-)
> 

Reviewed-by: Erik Skultety 



Re: [libvirt PATCH 1/4] vbox: use g_new0 in vboxDumpAudio

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 12:22:23AM +0200, Ján Tomko wrote:
> Elimination of the positive conditions reduces
> the indentation by two levels.
> 
> Signed-off-by: Ján Tomko 
> ---

No g_new0 was introduced by this commit, so the subject should be adjusted to
reflect the true nature of the patch.

Reviewed-by: Erik Skultety 



Re: [PATCH] rpm: Enable Xen support on AArch64

2020-10-05 Thread Andrea Bolognani
On Sun, 2020-10-04 at 22:16 -0400, Neal Gompa wrote:
> +++ b/libvirt.spec.in
> +# Xen is available only on i386 x86_64 ia64 aarch64
> +%ifnarch %{ix86} x86_64 ia64 aarch64
>  %define with_libxl 0
>  %endif

The code change is okay, but duplicating the list of architectures in
the comment is kinda pointless and results in unnecessary churn; I
would reword it as

  Xen is available only on some architectures

Are you okay with that change? If so, I will add my

  Reviewed-by: Andrea Bolognani 

and push the patch.

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [PATCH] rpm: Enable Xen support on AArch64

2020-10-05 Thread Ján Tomko

On a Sunday in 2020, Neal Gompa wrote:

Starting with Linux 5.9, Xen Dom0 works on commonly available
AArch64 devices, such as the Raspberry Pi 4.



My Fedora 32 does not even have 5.9 yet, does it make sense to allow it
regardless of the version?

Jano


Reference: https://xenproject.org/2020/09/29/xen-on-raspberry-pi-4-adventures/

Signed-off-by: Neal Gompa 
---
libvirt.spec.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index aa2bc84be9..470782b23e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -97,8 +97,8 @@

# Finally set the OS / architecture specific special cases

-# Xen is available only on i386 x86_64 ia64
-%ifnarch %{ix86} x86_64 ia64
+# Xen is available only on i386 x86_64 ia64 aarch64
+%ifnarch %{ix86} x86_64 ia64 aarch64
%define with_libxl 0
%endif

--
2.26.2



signature.asc
Description: PGP signature


[libvirt PATCH 8/9] qemu: firmware: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_firmware.c | 34 --
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 480ce0b00d..eebe6fcf78 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -301,8 +301,7 @@ qemuFirmwareInterfaceParse(const char *path,
 
 ninterfaces = virJSONValueArraySize(interfacesJSON);
 
-if (VIR_ALLOC_N(interfaces, ninterfaces) < 0)
-return -1;
+interfaces = g_new0(qemuFirmwareOSInterface, ninterfaces);
 
 for (i = 0; i < ninterfaces; i++) {
 virJSONValuePtr item = virJSONValueArrayGet(interfacesJSON, i);
@@ -504,8 +503,7 @@ qemuFirmwareTargetParse(const char *path,
 
 ntargets = virJSONValueArraySize(targetsJSON);
 
-if (VIR_ALLOC_N(targets, ntargets) < 0)
-return -1;
+targets = g_new0(qemuFirmwareTargetPtr, ntargets);
 
 for (i = 0; i < ntargets; i++) {
 virJSONValuePtr item = virJSONValueArrayGet(targetsJSON, i);
@@ -515,8 +513,7 @@ qemuFirmwareTargetParse(const char *path,
 size_t nmachines;
 size_t j;
 
-if (VIR_ALLOC(t) < 0)
-goto cleanup;
+t = g_new0(qemuFirmwareTarget, 1);
 
 if (!(architectureStr = virJSONValueObjectGetString(item, 
"architecture"))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -541,8 +538,7 @@ qemuFirmwareTargetParse(const char *path,
 
 nmachines = virJSONValueArraySize(machines);
 
-if (VIR_ALLOC_N(t->machines, nmachines) < 0)
-goto cleanup;
+t->machines = g_new0(char *, nmachines);
 
 for (j = 0; j < nmachines; j++) {
 virJSONValuePtr machine = virJSONValueArrayGet(machines, j);
@@ -588,8 +584,7 @@ qemuFirmwareFeatureParse(const char *path,
 
 nfeatures = virJSONValueArraySize(featuresJSON);
 
-if (VIR_ALLOC_N(features, nfeatures) < 0)
-return -1;
+features = g_new0(qemuFirmwareFeature, nfeatures);
 
 for (i = 0; i < nfeatures; i++) {
 virJSONValuePtr item = virJSONValueArrayGet(featuresJSON, i);
@@ -632,8 +627,7 @@ qemuFirmwareParse(const char *path)
 return NULL;
 }
 
-if (VIR_ALLOC(fw) < 0)
-return NULL;
+fw = g_new0(qemuFirmware, 1);
 
 if (qemuFirmwareInterfaceParse(path, doc, fw) < 0)
 return NULL;
@@ -1050,9 +1044,8 @@ qemuFirmwareEnableFeatures(virQEMUDriverPtr driver,
 
 switch (fw->mapping.device) {
 case QEMU_FIRMWARE_DEVICE_FLASH:
-if (!def->os.loader &&
-VIR_ALLOC(def->os.loader) < 0)
-return -1;
+if (!def->os.loader)
+def->os.loader = g_new0(virDomainLoaderDef, 1);
 
 def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
 def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
@@ -1093,9 +1086,8 @@ qemuFirmwareEnableFeatures(virQEMUDriverPtr driver,
 break;
 
 case QEMU_FIRMWARE_DEVICE_MEMORY:
-if (!def->os.loader &&
-VIR_ALLOC(def->os.loader) < 0)
-return -1;
+if (!def->os.loader)
+def->os.loader = g_new0(virDomainLoaderDef, 1);
 
 def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM;
 def->os.loader->path = g_strdup(memory->filename);
@@ -1201,8 +1193,7 @@ qemuFirmwareFetchParsedConfigs(bool privileged,
 
 npaths = virStringListLength((const char **)paths);
 
-if (VIR_ALLOC_N(firmwares, npaths) < 0)
-return -1;
+firmwares = g_new0(qemuFirmwarePtr, npaths);
 
 for (i = 0; i < npaths; i++) {
 if (!(firmwares[i] = qemuFirmwareParse(paths[i])))
@@ -1431,8 +1422,7 @@ qemuFirmwareGetSupported(const char *machine,
 }
 
 if (j == *nfws) {
-if (VIR_ALLOC(tmp) < 0)
-return -1;
+tmp = g_new0(virFirmware, 1);
 
 tmp->name = g_strdup(fwpath);
 tmp->nvram = g_strdup(nvrampath);
-- 
2.26.2



[libvirt PATCH 9/9] qemu: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_agent.c  |  6 ++
 src/qemu/qemu_block.c  | 15 +--
 src/qemu/qemu_cgroup.c |  6 ++
 src/qemu/qemu_conf.c   | 20 
 src/qemu/qemu_domain_address.c |  3 +--
 src/qemu/qemu_hotplug.c| 29 +++--
 src/qemu/qemu_migration.c  |  6 ++
 src/qemu/qemu_monitor.c| 13 +++--
 src/qemu/qemu_namespace.c  |  3 +--
 src/qemu/qemu_saveimage.c  |  3 +--
 src/qemu/qemu_vhost_user.c | 17 +++--
 11 files changed, 43 insertions(+), 78 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 1239aceb46..09116e749c 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1452,8 +1452,7 @@ qemuAgentGetVCPUs(qemuAgentPtr agent,
 
 ndata = virJSONValueArraySize(data);
 
-if (VIR_ALLOC_N(*info, ndata) < 0)
-goto cleanup;
+*info = g_new0(qemuAgentCPUInfo, ndata);
 
 for (i = 0; i < ndata; i++) {
 virJSONValuePtr entry = virJSONValueArrayGet(data, i);
@@ -2148,8 +2147,7 @@ qemuAgentGetInterfaces(qemuAgentPtr agent,
 if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
 goto error;
 
-if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
-goto error;
+ifaces_ret[ifaces_count - 1] = g_new0(virDomainInterface, 1);
 
 if (virHashAddEntry(ifaces_store, ifname_s,
 ifaces_ret[ifaces_count - 1]) < 0)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 487b0a72e7..6bea21347a 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -169,8 +169,7 @@ qemuBlockNodeNameGetBackingChainBacking(virJSONValuePtr 
next,
 }
 }
 
-if (VIR_ALLOC(data) < 0)
-return -1;
+data = g_new0(qemuBlockNodeNameBackingChainData, 1);
 
 data->nodeformat = g_strdup(nodename);
 data->nodestorage = g_strdup(parentnodename);
@@ -415,8 +414,7 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src)
 return NULL;
 }
 
-if (VIR_ALLOC(uri) < 0)
-return NULL;
+uri = g_new0(virURI, 1);
 
 if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
 uri->port = src->hosts->port;
@@ -1595,8 +1593,7 @@ 
qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src,
 if (autoreadonly)
 backendpropsflags |= 
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_AUTO_READONLY;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceAttachData, 1);
 
 if (!(data->formatProps = qemuBlockStorageSourceGetBlockdevProps(src,
  
backingStore)) ||
@@ -1886,8 +1883,7 @@ 
qemuBlockStorageSourceChainDetachPrepareBlockdev(virStorageSourcePtr src)
 g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
 virStorageSourcePtr n;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceChainData, 1);
 
 for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
 if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL)))
@@ -1916,8 +1912,7 @@ 
qemuBlockStorageSourceChainDetachPrepareDrive(virStorageSourcePtr src,
 g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL;
 g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceChainData, 1);
 
 if (!(backend = qemuBlockStorageSourceDetachPrepare(src, driveAlias)))
 return NULL;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e88da02341..b671991ad6 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -926,8 +926,7 @@ qemuInitCgroup(virDomainObjPtr vm,
 if (!vm->def->resource) {
 virDomainResourceDefPtr res;
 
-if (VIR_ALLOC(res) < 0)
-return -1;
+res = g_new0(virDomainResourceDef, 1);
 
 res->partition = g_strdup("/machine");
 
@@ -1252,8 +1251,7 @@ qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup,
 if (!(all_nodes_str = virBitmapFormat(all_nodes)))
 goto cleanup;
 
-if (VIR_ALLOC(data) < 0)
-goto cleanup;
+data = g_new0(qemuCgroupEmulatorAllNodesData, 1);
 
 if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
false, &data->emulatorCgroup) < 0)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f4ed1fa061..6d16c11017 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -641,9 +641,8 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr 
cfg,
 VIR_FREE(cfg->hugetlbfs);
 
 cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs);
-if (hugetlbfs[0] &&
-VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0)
-return -1;
+if (hugetlbfs[0])
+cfg->hugetlbf

Re: [libvirt PATCH] vmware: use g_new0 instead of VIR_ALLOC (glib chronicles)

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 12:23:25AM +0200, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---

Reviewed-by: Erik Skultety 



[libvirt PATCH 1/9] qemu: separate out VIR_ALLOC calls

2020-10-05 Thread Ján Tomko
Move them to separate conditions to reduce churn
in following patches.

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_domain.c   |  6 --
 src/qemu/qemu_monitor_json.c |  6 --
 src/qemu/qemu_process.c  | 23 ---
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0331fd55e0..491fb0ed3d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3719,8 +3719,10 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 
 if (j == def->npanics) {
 virDomainPanicDefPtr panic;
-if (VIR_ALLOC(panic) < 0 ||
-VIR_APPEND_ELEMENT_COPY(def->panics,
+if (VIR_ALLOC(panic) < 0)
+return -1;
+
+if (VIR_APPEND_ELEMENT_COPY(def->panics,
 def->npanics, panic) < 0) {
 VIR_FREE(panic);
 return -1;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bc242c798b..c399100dbe 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4890,8 +4890,10 @@ qemuMonitorJSONParseBlockJobInfo(virHashTablePtr 
blockJobs,
 if (!rawjobname)
 device = qemuAliasDiskDriveSkipPrefix(device);
 
-if (VIR_ALLOC(info) < 0 ||
-virHashAddEntry(blockJobs, device, info) < 0) {
+if (VIR_ALLOC(info) < 0)
+return -1;
+
+if (virHashAddEntry(blockJobs, device, info) < 0) {
 VIR_FREE(info);
 return -1;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9122069cc9..93cbb37986 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -832,17 +832,18 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 
 if (vm->def->watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) {
 struct qemuProcessEvent *processEvent;
-if (VIR_ALLOC(processEvent) == 0) {
-processEvent->eventType = QEMU_PROCESS_EVENT_WATCHDOG;
-processEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP;
-/* Hold an extra reference because we can't allow 'vm' to be
- * deleted before handling watchdog event is finished.
- */
-processEvent->vm = virObjectRef(vm);
-if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) 
{
-virObjectUnref(vm);
-qemuProcessEventFree(processEvent);
-}
+if (VIR_ALLOC(processEvent) < 0)
+;
+
+processEvent->eventType = QEMU_PROCESS_EVENT_WATCHDOG;
+processEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP;
+/* Hold an extra reference because we can't allow 'vm' to be
+ * deleted before handling watchdog event is finished.
+ */
+processEvent->vm = virObjectRef(vm);
+if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
+virObjectUnref(vm);
+qemuProcessEventFree(processEvent);
 }
 }
 
-- 
2.26.2



[libvirt PATCH 4/9] qemu: driver: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_driver.c | 62 ++
 1 file changed, 21 insertions(+), 41 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85b6a6a321..e622da56bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -625,8 +625,7 @@ qemuStateInitialize(bool privileged,
 const char *defsecmodel = NULL;
 g_autofree virSecurityManagerPtr *sec_managers = NULL;
 
-if (VIR_ALLOC(qemu_driver) < 0)
-return VIR_DRV_STATE_INIT_ERROR;
+qemu_driver = g_new0(virQEMUDriver, 1);
 
 qemu_driver->lockFD = -1;
 
@@ -1059,8 +1058,7 @@ qemuStateStop(void)

VIR_CONNECT_LIST_DOMAINS_ACTIVE)) < 0)
 goto cleanup;
 
-if (VIR_ALLOC_N(flags, numDomains) < 0)
-goto cleanup;
+flags = g_new0(unsigned int, numDomains);
 
 /* First we pause all VMs to make them stop dirtying
pages, etc. We remember if any VMs were paused so
@@ -5043,14 +5041,12 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
 goto endjob;
 }
 
-if (VIR_ALLOC_N(info_ret, niothreads) < 0)
-goto endjob;
+info_ret = g_new0(virDomainIOThreadInfoPtr, niothreads);
 
 for (i = 0; i < niothreads; i++) {
 virBitmapPtr map = NULL;
 
-if (VIR_ALLOC(info_ret[i]) < 0)
-goto endjob;
+info_ret[i] = g_new0(virDomainIOThreadInfo, 1);
 info_ret[i]->iothread_id = iothreads[i]->iothread_id;
 
 if (!(map = virProcessGetAffinity(iothreads[i]->thread_id)))
@@ -5098,12 +5094,10 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
 if (targetDef->niothreadids == 0)
 return 0;
 
-if (VIR_ALLOC_N(info_ret, targetDef->niothreadids) < 0)
-goto cleanup;
+info_ret = g_new0(virDomainIOThreadInfoPtr, targetDef->niothreadids);
 
 for (i = 0; i < targetDef->niothreadids; i++) {
-if (VIR_ALLOC(info_ret[i]) < 0)
-goto cleanup;
+info_ret[i] = g_new0(virDomainIOThreadInfo, 1);
 
 /* IOThread ID's are taken from the iothreadids list */
 info_ret[i]->iothread_id = targetDef->iothreadids[i]->iothread_id;
@@ -5945,10 +5939,7 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr 
dom,
 for (i = 0; mgrs[i]; i++)
 len++;
 
-if (VIR_ALLOC_N((*seclabels), len) < 0) {
-VIR_FREE(mgrs);
-goto cleanup;
-}
+(*seclabels) = g_new0(virSecurityLabel, len);
 memset(*seclabels, 0, sizeof(**seclabels) * len);
 
 /* Fill the array */
@@ -9979,8 +9970,7 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
 if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0 || rc < 0)
 goto cleanup;
 
-if (VIR_ALLOC(*retstats) < 0)
-goto cleanup;
+*retstats = g_new0(qemuBlockStats, 1);
 
 if (entryname) {
 if (!(stats = virHashLookup(blockstats, entryname))) {
@@ -10284,10 +10274,9 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 goto endjob;
 }
 
-if ((VIR_ALLOC(bandwidth) < 0) ||
-(VIR_ALLOC(bandwidth->in) < 0) ||
-(VIR_ALLOC(bandwidth->out) < 0))
-goto endjob;
+bandwidth = g_new0(virNetDevBandwidth, 1);
+bandwidth->in = g_new0(virNetDevBandwidthRate, 1);
+bandwidth->out = g_new0(virNetDevBandwidthRate, 1);
 
 for (i = 0; i < nparams; i++) {
 virTypedParameterPtr param = ¶ms[i];
@@ -10321,16 +10310,14 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 VIR_FREE(bandwidth->out);
 
 if (net) {
-if (VIR_ALLOC(newBandwidth) < 0)
-goto endjob;
+newBandwidth = g_new0(virNetDevBandwidth, 1);
 
 /* virNetDevBandwidthSet() will clear any previous value of
  * bandwidth parameters, so merge with old bandwidth parameters
  * here to prevent them from being lost. */
 if (bandwidth->in ||
 (!inboundSpecified && net->bandwidth && net->bandwidth->in)) {
-if (VIR_ALLOC(newBandwidth->in) < 0)
-goto endjob;
+newBandwidth->in = g_new0(virNetDevBandwidthRate, 1);
 
 memcpy(newBandwidth->in,
bandwidth->in ? bandwidth->in : net->bandwidth->in,
@@ -10338,8 +10325,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 }
 if (bandwidth->out ||
 (!outboundSpecified && net->bandwidth && net->bandwidth->out)) {
-if (VIR_ALLOC(newBandwidth->out) < 0)
-goto endjob;
+newBandwidth->out = g_new0(virNetDevBandwidthRate, 1);
 
 memcpy(newBandwidth->out,
bandwidth->out ? bandwidth->out : net->bandwidth->out,
@@ -12463,8 +12449,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
 if (qemuProcessQMPStart(proc) < 0)
 return NULL;
 
-if (VIR_ALLOC(baseline) < 0)
-return NULL;
+baseline = g_new0(virCPUDef, 1);
 
 if 

[libvirt PATCH 0/9] qemu: use g_new0 (glib chronicles)

2020-10-05 Thread Ján Tomko
With the exception of qemu_migrate_cookie and qemu_slirp which
should be dealt with by Peter's series.

Ján Tomko (9):
  qemu: separate out VIR_ALLOC calls
  qemu: capabilities: use g_new0
  qemu: domain: use g_new0
  qemu: driver: use g_new0
  qemu: monitor: json: use g_new0
  qemu: process: use g_new0
  qemu: command: use g_new0
  qemu: firmware: use g_new0
  qemu: use g_new0

 src/qemu/qemu_agent.c  |   6 +-
 src/qemu/qemu_block.c  |  15 ++---
 src/qemu/qemu_capabilities.c   |  48 +-
 src/qemu/qemu_cgroup.c |   6 +-
 src/qemu/qemu_command.c|  35 ---
 src/qemu/qemu_conf.c   |  20 +++---
 src/qemu/qemu_domain.c |  55 ++--
 src/qemu/qemu_domain_address.c |   3 +-
 src/qemu/qemu_driver.c |  62 +++---
 src/qemu/qemu_firmware.c   |  34 --
 src/qemu/qemu_hotplug.c|  29 -
 src/qemu/qemu_migration.c  |   6 +-
 src/qemu/qemu_monitor.c|  13 +---
 src/qemu/qemu_monitor_json.c   | 112 +++--
 src/qemu/qemu_namespace.c  |   3 +-
 src/qemu/qemu_process.c|  75 +-
 src/qemu/qemu_saveimage.c  |   3 +-
 src/qemu/qemu_vhost_user.c |  17 +++--
 18 files changed, 190 insertions(+), 352 deletions(-)

-- 
2.26.2



[libvirt PATCH 3/9] qemu: domain: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_domain.c | 53 ++
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 491fb0ed3d..15912317de 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -496,8 +496,7 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
 goto error;
 }
 
-if (VIR_ALLOC_N(masterKey, 1024) < 0)
-goto error;
+masterKey = g_new0(uint8_t, 1024);
 
 if ((masterKeyLen = saferead(fd, masterKey, 1024)) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -572,8 +571,7 @@ qemuDomainMasterKeyCreate(virDomainObjPtr vm)
 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET))
 return 0;
 
-if (VIR_ALLOC_N(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
-return -1;
+priv->masterKey = g_new0(uint8_t, QEMU_DOMAIN_MASTER_KEY_LEN);
 priv->masterKeyLen = QEMU_DOMAIN_MASTER_KEY_LEN;
 
 if (virRandomBytes(priv->masterKey, priv->masterKeyLen) < 0) {
@@ -1188,8 +1186,7 @@ qemuDomainSecretInfoNewPlain(virSecretUsageType usageType,
 {
 qemuDomainSecretInfoPtr secinfo = NULL;
 
-if (VIR_ALLOC(secinfo) < 0)
-return NULL;
+secinfo = g_new0(qemuDomainSecretInfo, 1);
 
 if (qemuDomainSecretPlainSetup(secinfo, usageType, username, lookupDef) < 
0) {
 g_clear_pointer(&secinfo, qemuDomainSecretInfoFree);
@@ -1689,8 +1686,7 @@ qemuDomainObjPrivateAlloc(void *opaque)
 {
 qemuDomainObjPrivatePtr priv;
 
-if (VIR_ALLOC(priv) < 0)
-return NULL;
+priv = g_new0(qemuDomainObjPrivate, 1);
 
 if (qemuDomainObjInitJob(&priv->job, &qemuPrivateJobCallbacks) < 0) {
 virReportSystemError(errno, "%s",
@@ -1845,9 +1841,7 @@ 
qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfoPtr *secinfo,
 return 0;
 
 if (!*secinfo) {
-if (VIR_ALLOC(*secinfo) < 0)
-return -1;
-
+*secinfo = g_new0(qemuDomainSecretInfo, 1);
 (*secinfo)->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES;
 }
 
@@ -3083,8 +3077,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 }
 if (n > 0) {
 /* NULL-terminated list */
-if (VIR_ALLOC_N(priv->qemuDevices, n + 1) < 0)
-goto error;
+priv->qemuDevices = g_new0(char *, n + 1);
 
 for (i = 0; i < n; i++) {
 priv->qemuDevices[i] = virXMLPropString(nodes[i], "alias");
@@ -3245,8 +3238,7 @@ 
qemuDomainDefNamespaceParseCommandlineArgs(qemuDomainXmlNsDefPtr nsdef,
 if (nnodes == 0)
 return 0;
 
-if (VIR_ALLOC_N(nsdef->args, nnodes) < 0)
-return -1;
+nsdef->args = g_new0(char *, nnodes);
 
 for (i = 0; i < nnodes; i++) {
 if (!(nsdef->args[nsdef->num_args++] = virXMLPropString(nodes[i], 
"value"))) {
@@ -3293,9 +3285,8 @@ 
qemuDomainDefNamespaceParseCommandlineEnv(qemuDomainXmlNsDefPtr nsdef,
 if (nnodes == 0)
 return 0;
 
-if (VIR_ALLOC_N(nsdef->env_name, nnodes) < 0 ||
-VIR_ALLOC_N(nsdef->env_value, nnodes) < 0)
-return -1;
+nsdef->env_name = g_new0(char *, nnodes);
+nsdef->env_value = g_new0(char *, nnodes);
 
 for (i = 0; i < nnodes; i++) {
 if (!(nsdef->env_name[nsdef->num_env] = virXMLPropString(nodes[i], 
"name"))) {
@@ -3331,8 +3322,7 @@ qemuDomainDefNamespaceParseCaps(qemuDomainXmlNsDefPtr 
nsdef,
 return -1;
 
 if (nnodesadd > 0) {
-if (VIR_ALLOC_N(nsdef->capsadd, nnodesadd) < 0)
-return -1;
+nsdef->capsadd = g_new0(char *, nnodesadd);
 
 for (i = 0; i < nnodesadd; i++) {
 if (!(nsdef->capsadd[nsdef->ncapsadd++] = 
virXMLPropString(nodesadd[i], "capability"))) {
@@ -3344,8 +3334,7 @@ qemuDomainDefNamespaceParseCaps(qemuDomainXmlNsDefPtr 
nsdef,
 }
 
 if (nnodesdel > 0) {
-if (VIR_ALLOC_N(nsdef->capsdel, nnodesdel) < 0)
-return -1;
+nsdef->capsdel = g_new0(char *, nnodesdel);
 
 for (i = 0; i < nnodesdel; i++) {
 if (!(nsdef->capsdel[nsdef->ncapsdel++] = 
virXMLPropString(nodesdel[i], "capability"))) {
@@ -3367,8 +3356,7 @@ qemuDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
 qemuDomainXmlNsDefPtr nsdata = NULL;
 int ret = -1;
 
-if (VIR_ALLOC(nsdata) < 0)
-return -1;
+nsdata = g_new0(qemuDomainXmlNsDef, 1);
 
 if (qemuDomainDefNamespaceParseCommandlineArgs(nsdata, ctxt) < 0 ||
 qemuDomainDefNamespaceParseCommandlineEnv(nsdata, ctxt) < 0 ||
@@ -3661,8 +3649,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 
 if (addDefaultMemballoon && !def->memballoon) {
 virDomainMemballoonDefPtr memballoon;
-if (VIR_ALLOC(memballoon) < 0)
-return -1;
+memballoon = g_new0(virDomainMemballoonDef, 1);
 
 memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO;
 def->memballoon = memballoon;
@@ -3718,9 +3705,7 @@ qemuDomainDefAddDefaultDevic

[libvirt PATCH 2/9] qemu: capabilities: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_capabilities.c | 48 
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5dcfcd574d..2e0e4492a4 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -943,14 +943,12 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
 *machines = NULL;
 *nmachines = accel->nmachineTypes;
 
-if (*nmachines &&
-VIR_ALLOC_N(*machines, accel->nmachineTypes) < 0)
-goto error;
+if (*nmachines)
+*machines = g_new0(virCapsGuestMachinePtr, accel->nmachineTypes);
 
 for (i = 0; i < accel->nmachineTypes; i++) {
 virCapsGuestMachinePtr mach;
-if (VIR_ALLOC(mach) < 0)
-goto error;
+mach = g_new0(virCapsGuestMachine, 1);
 (*machines)[i] = mach;
 if (accel->machineTypes[i].alias) {
 mach->name = g_strdup(accel->machineTypes[i].alias);
@@ -985,8 +983,7 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
 
 if (!found) {
 virCapsGuestMachinePtr mach;
-if (VIR_ALLOC(mach) < 0)
-goto error;
+mach = g_new0(virCapsGuestMachine, 1);
 if (VIR_INSERT_ELEMENT_COPY(*machines, i, *nmachines, mach) < 0) {
 VIR_FREE(mach);
 goto error;
@@ -1868,8 +1865,7 @@ virQEMUCapsSEVInfoCopy(virSEVCapabilityPtr *dst,
 {
 g_autoptr(virSEVCapability) tmp = NULL;
 
-if (VIR_ALLOC(tmp) < 0)
-return -1;
+tmp = g_new0(virSEVCapability, 1);
 
 tmp->pdh = g_strdup(src->pdh);
 tmp->cert_chain = g_strdup(src->cert_chain);
@@ -1949,8 +1945,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
 virQEMUCapsAccelCopy(&ret->tcg, &qemuCaps->tcg) < 0)
 goto error;
 
-if (VIR_ALLOC_N(ret->gicCapabilities, qemuCaps->ngicCapabilities) < 0)
-goto error;
+ret->gicCapabilities = g_new0(virGICCapability, 
qemuCaps->ngicCapabilities);
 ret->ngicCapabilities = qemuCaps->ngicCapabilities;
 for (i = 0; i < qemuCaps->ngicCapabilities; i++)
 ret->gicCapabilities[i] = qemuCaps->gicCapabilities[i];
@@ -3184,8 +3179,7 @@ virQEMUCapsGetCPUFeatures(virQEMUCapsPtr qemuCaps,
 if (!modelInfo)
 return 0;
 
-if (VIR_ALLOC_N(list, modelInfo->nprops + 1) < 0)
-return -1;
+list = g_new0(char *, modelInfo->nprops + 1);
 
 n = 0;
 for (i = 0; i < modelInfo->nprops; i++) {
@@ -3561,8 +3555,7 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
 }
 
 cpu->model = g_strdup(modelInfo->name);
-if (VIR_ALLOC_N(cpu->features, modelInfo->nprops) < 0)
-return -1;
+cpu->features = g_new0(virCPUFeatureDef, modelInfo->nprops);
 
 cpu->nfeatures_max = modelInfo->nprops;
 cpu->nfeatures = 0;
@@ -3872,8 +3865,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccelPtr caps,
 goto cleanup;
 }
 
-if (VIR_ALLOC(hostCPU) < 0)
-goto cleanup;
+hostCPU = g_new0(qemuMonitorCPUModelInfo, 1);
 
 if (!(hostCPU->name = virXMLPropString(hostCPUNode, "model"))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -3894,9 +3886,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccelPtr caps,
 ctxt->node = hostCPUNode;
 
 if ((n = virXPathNodeSet("./property", ctxt, &nodes)) > 0) {
-if (VIR_ALLOC_N(hostCPU->props, n) < 0)
-goto cleanup;
-
+hostCPU->props = g_new0(qemuMonitorCPUProperty, n);
 hostCPU->nprops = n;
 
 for (i = 0; i < n; i++) {
@@ -4041,8 +4031,7 @@ virQEMUCapsLoadCPUModels(virQEMUCapsAccelPtr caps,
 if (nblockers > 0) {
 size_t j;
 
-if (VIR_ALLOC_N(cpu->blockers, nblockers + 1) < 0)
-return -1;
+cpu->blockers = g_new0(char *, nblockers + 1);
 
 for (j = 0; j < nblockers; j++) {
 if (!(cpu->blockers[j] = virXMLPropString(blockerNodes[j], 
"name"))) {
@@ -4081,8 +4070,7 @@ virQEMUCapsLoadMachines(virQEMUCapsAccelPtr caps,
 return 0;
 
 caps->nmachineTypes = n;
-if (VIR_ALLOC_N(caps->machineTypes, caps->nmachineTypes) < 0)
-return -1;
+caps->machineTypes = g_new0(virQEMUCapsMachineType, caps->nmachineTypes);
 
 for (i = 0; i < n; i++) {
 if (!(caps->machineTypes[i].name = virXMLPropString(nodes[i], 
"name"))) {
@@ -4189,8 +4177,7 @@ virQEMUCapsParseSEVInfo(virQEMUCapsPtr qemuCaps, 
xmlXPathContextPtr ctxt)
 return -1;
 }
 
-if (VIR_ALLOC(sev) < 0)
-return -1;
+sev = g_new0(virSEVCapability, 1);
 
 if (virXPathUInt("string(./sev/cbitpos)", ctxt, &sev->cbitpos) < 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -4408,8 +4395,7 @@ virQEMUCapsLoadCache(virArch hostArch,
 bool boolValue;
 
 qemuCaps->ngicCapabilities = n;
-if (VIR_ALLOC_N(qemuCaps->gicCapabilities, n) < 0)
-goto cleanup;

[libvirt PATCH 5/9] qemu: monitor: json: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_monitor_json.c | 110 ---
 1 file changed, 36 insertions(+), 74 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index c399100dbe..26ac499fc5 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -769,8 +769,7 @@ qemuMonitorJSONGuestPanicExtractInfoHyperv(virJSONValuePtr 
data)
 {
 qemuMonitorEventPanicInfoPtr ret;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(qemuMonitorEventPanicInfo, 1);
 
 ret->type = QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV;
 
@@ -799,8 +798,7 @@ qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValuePtr 
data)
 unsigned long long psw_mask, psw_addr;
 const char *reason = NULL;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(qemuMonitorEventPanicInfo, 1);
 
 ret->type = QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390;
 
@@ -1900,8 +1898,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
 if ((ncpus = virJSONValueArraySize(data)) == 0)
 return -2;
 
-if (VIR_ALLOC_N(cpus, ncpus) < 0)
-goto cleanup;
+cpus = g_new0(struct qemuMonitorQueryCpusEntry, ncpus);
 
 for (i = 0; i < ncpus; i++) {
 virJSONValuePtr entry = virJSONValueArrayGet(data, i);
@@ -2438,8 +2435,7 @@ qemuMonitorJSONBlockInfoAdd(virHashTablePtr table,
 struct qemuDomainDiskInfo *tmp = NULL;
 int ret = -1;
 
-if (VIR_ALLOC(tmp) < 0)
-goto cleanup;
+tmp = g_new0(struct qemuDomainDiskInfo, 1);
 
 *tmp = *info;
 tmp->nodename = NULL;
@@ -2555,8 +2551,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev,
 return NULL;
 }
 
-if (VIR_ALLOC(bstats) < 0)
-return NULL;
+bstats = g_new0(qemuBlockStats, 1);
 
 #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \
 if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \
@@ -2595,8 +2590,7 @@ qemuMonitorJSONAddOneBlockStatsInfo(qemuBlockStatsPtr 
bstats,
 {
 qemuBlockStatsPtr copy = NULL;
 
-if (VIR_ALLOC(copy) < 0)
-return -1;
+copy = g_new0(qemuBlockStats, 1);
 
 if (bstats)
 *copy = *bstats;
@@ -2744,8 +2738,7 @@ 
qemuMonitorJSONBlockStatsUpdateCapacityData(virJSONValuePtr image,
 qemuBlockStatsPtr bstats;
 
 if (!(bstats = virHashLookup(stats, name))) {
-if (VIR_ALLOC(bstats) < 0)
-return -1;
+bstats = g_new0(qemuBlockStats, 1);
 
 if (virHashAddEntry(stats, name, bstats) < 0) {
 VIR_FREE(bstats);
@@ -4104,8 +4097,7 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
 goto cleanup;
 }
 nTable = virJSONValueArraySize(table);
-if (VIR_ALLOC_N(fil->unicast.table, nTable))
-goto cleanup;
+fil->unicast.table = g_new0(virMacAddr, nTable);
 for (i = 0; i < nTable; i++) {
 if (!(element = virJSONValueArrayGet(table, i)) ||
 !(tmp = virJSONValueGetString(element))) {
@@ -4146,8 +4138,7 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
 goto cleanup;
 }
 nTable = virJSONValueArraySize(table);
-if (VIR_ALLOC_N(fil->multicast.table, nTable))
-goto cleanup;
+fil->multicast.table = g_new0(virMacAddr, nTable);
 for (i = 0; i < nTable; i++) {
 if (!(element = virJSONValueArrayGet(table, i)) ||
 !(tmp = virJSONValueGetString(element))) {
@@ -4181,8 +4172,7 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
 goto cleanup;
 }
 nTable = virJSONValueArraySize(table);
-if (VIR_ALLOC_N(fil->vlan.table, nTable))
-goto cleanup;
+fil->vlan.table = g_new0(unsigned int, nTable);
 for (i = 0; i < nTable; i++) {
 if (!(element = virJSONValueArrayGet(table, i)) ||
 virJSONValueGetNumberUint(element, &fil->vlan.table[i]) < 0) {
@@ -4284,8 +4274,7 @@ qemuMonitorJSONExtractChardevInfo(virJSONValuePtr reply,
 goto cleanup;
 }
 
-if (VIR_ALLOC(entry) < 0)
-goto cleanup;
+entry = g_new0(qemuMonitorChardevInfo, 1);
 
 if (STRPREFIX(type, "pty:"))
 entry->ptyPath = g_strdup(type + strlen("pty:"));
@@ -4890,8 +4879,7 @@ qemuMonitorJSONParseBlockJobInfo(virHashTablePtr 
blockJobs,
 if (!rawjobname)
 device = qemuAliasDiskDriveSkipPrefix(device);
 
-if (VIR_ALLOC(info) < 0)
-return -1;
+info = g_new0(qemuMonitorBlockJobInfo, 1);
 
 if (virHashAddEntry(blockJobs, device, info) < 0) {
 VIR_FREE(info);
@@ -5576,16 +5564,14 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon,
 n = virJSONValueArraySize(data);
 
 /* null-terminated list */
-if (VIR_ALLOC_N(infolist, n + 1) < 0)
-goto cleanup;
+infolist = g_new0(qemuMonitorMachineInfoPtr, n + 1);
 
 for (i = 0; i < n; i++) {
 virJSONValuePtr child = virJSONValueArrayGet(data, i);
 const char *tmp;
 qemuMonitorMachineInfoPtr info;

[libvirt PATCH 7/9] qemu: command: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_command.c | 35 +--
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e9ba81d82f..476cf6972e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7428,8 +7428,7 @@ qemuBuildNumaCommandLine(virQEMUDriverConfigPtr cfg,
 hmat = true;
 }
 
-if (VIR_ALLOC_N(nodeBackends, ncells) < 0)
-goto cleanup;
+nodeBackends = g_new0(virBuffer, ncells);
 
 /* using of -numa memdev= cannot be combined with -numa mem=, thus we
  * need to check which approach to use */
@@ -8119,9 +8118,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 if (!tapfdSize)
 tapfdSize = 1;
 
-if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 ||
-VIR_ALLOC_N(tapfdName, tapfdSize) < 0)
-goto cleanup;
+tapfd = g_new0(int, tapfdSize);
+tapfdName = g_new0(char *, tapfdSize);
 
 memset(tapfd, -1, tapfdSize * sizeof(tapfd[0]));
 
@@ -8135,9 +8133,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 if (!tapfdSize)
 tapfdSize = 1;
 
-if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 ||
-VIR_ALLOC_N(tapfdName, tapfdSize) < 0)
-goto cleanup;
+tapfd = g_new0(int, tapfdSize);
+tapfdName = g_new0(char *, tapfdSize);
 
 memset(tapfd, -1, tapfdSize * sizeof(tapfd[0]));
 
@@ -8151,9 +8148,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 if (!tapfdSize)
 tapfdSize = 1;
 
-if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 ||
-VIR_ALLOC_N(tapfdName, tapfdSize) < 0)
-goto cleanup;
+tapfd = g_new0(int, tapfdSize);
+tapfdName = g_new0(char *, tapfdSize);
 
 memset(tapfd, -1, tapfdSize * sizeof(tapfd[0]));
 
@@ -8268,9 +8264,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 if (!vhostfdSize)
 vhostfdSize = 1;
 
-if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0 ||
-VIR_ALLOC_N(vhostfdName, vhostfdSize))
-goto cleanup;
+vhostfd = g_new0(int, vhostfdSize);
+vhostfdName = g_new0(char *, vhostfdSize);
 
 memset(vhostfd, -1, vhostfdSize * sizeof(vhostfd[0]));
 
@@ -10465,8 +10460,7 @@ 
qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk,
 {
 g_autoptr(qemuBlockStorageSourceAttachData) data = NULL;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceAttachData, 1);
 
 if (!(data->driveCmd = qemuBuildDriveStr(disk, qemuCaps)) ||
 !(data->driveAlias = qemuAliasDiskDriveFromDisk(disk)))
@@ -10543,8 +10537,7 @@ 
qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk,
 g_autoptr(qemuBlockStorageSourceAttachData) elem = NULL;
 g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceChainData, 1);
 
 if (!(elem = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps)))
 return NULL;
@@ -10595,8 +10588,7 @@ 
qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top,
 g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
 virStorageSourcePtr n;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceChainData, 1);
 
 for (n = top; virStorageSourceIsBacking(n); n = n->backingStore) {
 if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, n,
@@ -10625,8 +10617,7 @@ 
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSourcePtr top,
 {
 g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
 
-if (VIR_ALLOC(data) < 0)
-return NULL;
+data = g_new0(qemuBlockStorageSourceChainData, 1);
 
 if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, top, 
backingStore,
 qemuCaps) < 0)
-- 
2.26.2



[libvirt PATCH 6/9] qemu: process: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_process.c | 56 +
 1 file changed, 18 insertions(+), 38 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 93cbb37986..94b44f1c5c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -302,8 +302,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon,
 goto cleanup;
 }
 
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_MONITOR_EOF;
 processEvent->vm = virObjectRef(vm);
@@ -832,8 +831,7 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon G_GNUC_UNUSED,
 
 if (vm->def->watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) {
 struct qemuProcessEvent *processEvent;
-if (VIR_ALLOC(processEvent) < 0)
-;
+processEvent = g_new0(qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_WATCHDOG;
 processEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP;
@@ -965,8 +963,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon G_GNUC_UNUSED,
 virDomainObjBroadcast(vm);
 } else {
 /* there is no waiting SYNC API, dispatch the update to a thread */
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_BLOCK_JOB;
 data = g_strdup(diskAlias);
@@ -1030,8 +1027,7 @@ qemuProcessHandleJobStatusChange(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 virDomainObjBroadcast(vm);
 } else {
 VIR_DEBUG("job '%s' handled by event thread", jobname);
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE;
 processEvent->vm = virObjectRef(vm);
@@ -1074,20 +1070,17 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 virDomainEventGraphicsSubjectPtr subject = NULL;
 size_t i;
 
-if (VIR_ALLOC(localAddr) < 0)
-goto error;
+localAddr = g_new0(virDomainEventGraphicsAddress, 1);
 localAddr->family = localFamily;
 localAddr->service = g_strdup(localService);
 localAddr->node = g_strdup(localNode);
 
-if (VIR_ALLOC(remoteAddr) < 0)
-goto error;
+remoteAddr = g_new0(virDomainEventGraphicsAddress, 1);
 remoteAddr->family = remoteFamily;
 remoteAddr->service = g_strdup(remoteService);
 remoteAddr->node = g_strdup(remoteNode);
 
-if (VIR_ALLOC(subject) < 0)
-goto error;
+subject = g_new0(virDomainEventGraphicsSubject, 1);
 if (x509dname) {
 if (VIR_REALLOC_N(subject->identities, subject->nidentity+1) < 0)
 goto error;
@@ -1329,8 +1322,7 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 struct qemuProcessEvent *processEvent;
 
 virObjectLock(vm);
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_GUESTPANIC;
 processEvent->action = vm->def->onCrash;
@@ -1345,7 +1337,6 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 qemuProcessEventFree(processEvent);
 }
 
- cleanup:
 virObjectUnlock(vm);
 
 return 0;
@@ -1371,8 +1362,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon 
G_GNUC_UNUSED,
   QEMU_DOMAIN_UNPLUGGING_DEVICE_STATUS_OK))
 goto cleanup;
 
-if (VIR_ALLOC(processEvent) < 0)
-goto error;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_DEVICE_DELETED;
 data = g_strdup(devAlias);
@@ -1552,8 +1542,7 @@ qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 VIR_DEBUG("Device %s RX Filter changed in domain %p %s",
   devAlias, vm, vm->def->name);
 
-if (VIR_ALLOC(processEvent) < 0)
-goto error;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED;
 data = g_strdup(devAlias);
@@ -1590,8 +1579,7 @@ qemuProcessHandleSerialChanged(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 VIR_DEBUG("Serial port %s state changed to '%d' in domain %p %s",
   devAlias, connected, vm, vm->def->name);
 
-if (VIR_ALLOC(processEvent) < 0)
-goto error;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_SERIAL_CHANGED;
 data = g_strdup(devAlias);
@@ -1800,8 +1788,7 @@ qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr 
mon G_GNUC_UNUSED,
 priv = vm->privateData;
 priv->prDaemonRunning = false;
 
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->

[libvirt PATCH v2] docs: use "::" instead of ".. code-block::"

2020-10-05 Thread Daniel P . Berrangé
The former is a short hand for the latter and is already widely used in
the docs. Using the short hand avoids incompatibility with the alternate
impl of rst2html5.

Signed-off-by: Daniel P. Berrangé 
---

Changed in v2:

 - Simplify to use "::" instead of adding a language to
   ".. code-block::" because the latter didn't work on macOS
   builds due to problems with pygments integration in homebrew

 docs/manpages/libvirtd.rst |   4 +-
 docs/manpages/virsh.rst| 582 ++---
 docs/manpages/virt-admin.rst   |  52 +--
 docs/manpages/virt-login-shell.rst |   8 +-
 docs/manpages/virtlockd.rst|   4 +-
 docs/manpages/virtlogd.rst |   4 +-
 6 files changed, 327 insertions(+), 327 deletions(-)

diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst
index c88e2dd7e1..cd9976c5e4 100644
--- a/docs/manpages/libvirtd.rst
+++ b/docs/manpages/libvirtd.rst
@@ -199,7 +199,7 @@ EXAMPLES
 
 To retrieve the version of libvirtd:
 
-.. code-block::
+::
 
   # libvirtd --version
   libvirtd (libvirt) 0.8.2
@@ -207,7 +207,7 @@ To retrieve the version of libvirtd:
 
 To start libvirtd, instructing it to daemonize and create a PID file:
 
-.. code-block::
+::
 
   # libvirtd -d
   # ls -la @RUNSTATEDIR@/libvirtd.pid
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 9ac379a4b8..c855269041 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -38,7 +38,7 @@ KVM, LXC, OpenVZ, VirtualBox and VMware ESX.
 The basic structure of most virsh usage is:
 
 
-.. code-block::
+::
 
virsh [OPTION]...   [ARG]...
 
@@ -185,7 +185,7 @@ historical reasons, some commands default to bytes, while 
other
 commands default to kibibytes).  The following case-insensitive
 suffixes can be used to select a specific scale:
 
-.. code-block::
+::
 
b, byte  byte  1
KB   kilobyte  1,000
@@ -214,7 +214,7 @@ help
 
 **Syntax:**
 
-.. code-block::
+::
 
help [command-or-group]
 
@@ -228,7 +228,7 @@ group as an option.  For example:
 
 **Example 1:**
 
-.. code-block::
+::
 
virsh # help host
 
@@ -250,7 +250,7 @@ option instead.  For example:
 
 **Example 2:**
 
-.. code-block::
+::
 
virsh # help list
  NAME
@@ -272,7 +272,7 @@ quit, exit
 
 **Syntax:**
 
-.. code-block::
+::
 
quit
exit
@@ -286,7 +286,7 @@ version
 
 **Syntax:**
 
-.. code-block::
+::
 
version [--daemon]
 
@@ -296,7 +296,7 @@ is included in the output.
 
 **Example:**
 
-.. code-block::
+::
 
$ virsh version
Compiled against library: libvirt 1.2.3
@@ -317,7 +317,7 @@ cd
 
 **Syntax:**
 
-.. code-block::
+::
 
cd [directory]
 
@@ -334,7 +334,7 @@ pwd
 
 **Syntax:**
 
-.. code-block::
+::
 
pwd
 
@@ -347,7 +347,7 @@ connect
 
 **Syntax:**
 
-.. code-block::
+::
 
connect [URI] [--readonly]
 
@@ -388,7 +388,7 @@ uri
 
 **Syntax:**
 
-.. code-block::
+::
 
uri
 
@@ -400,7 +400,7 @@ hostname
 
 **Syntax:**
 
-.. code-block::
+::
 
hostname
 
@@ -412,7 +412,7 @@ sysinfo
 
 **Syntax:**
 
-.. code-block::
+::
 
sysinfo
 
@@ -424,7 +424,7 @@ nodeinfo
 
 **Syntax:**
 
-.. code-block::
+::
 
nodeinfo
 
@@ -440,7 +440,7 @@ nodecpumap
 
 **Syntax:**
 
-.. code-block::
+::
 
nodecpumap [--pretty]
 
@@ -456,7 +456,7 @@ nodecpustats
 
 **Syntax:**
 
-.. code-block::
+::
 
nodecpustats [cpu] [--percent]
 
@@ -471,7 +471,7 @@ nodememstats
 
 **Syntax:**
 
-.. code-block::
+::
 
nodememstats [cell]
 
@@ -484,7 +484,7 @@ nodesuspend
 
 **Syntax:**
 
-.. code-block::
+::
 
nodesuspend [target] [duration]
 
@@ -503,7 +503,7 @@ node-memory-tune
 
 **Syntax:**
 
-.. code-block::
+::
 
node-memory-tune [shm-pages-to-scan] [shm-sleep-millisecs] 
[shm-merge-across-nodes]
 
@@ -525,7 +525,7 @@ capabilities
 
 **Syntax:**
 
-.. code-block::
+::
 
capabilities
 
@@ -545,7 +545,7 @@ domcapabilities
 
 **Syntax:**
 
-.. code-block::
+::
 
domcapabilities [virttype] [emulatorbin] [arch] [machine]
 
@@ -588,7 +588,7 @@ pool-capabilities
 
 **Syntax:**
 
-.. code-block::
+::
 
pool-capabilities
 
@@ -605,7 +605,7 @@ inject-nmi
 
 **Syntax:**
 
-.. code-block::
+::
 
inject-nmi domain
 
@@ -617,7 +617,7 @@ list
 
 **Syntax:**
 
-.. code-block::
+::
 
list [--inactive | --all]
 [--managed-save] [--title]
@@ -637,7 +637,7 @@ specified it prints out information about running domains.
 
 An example format for the list is as follows:
 
-.. code-block::
+::
 
``virsh`` list
  IdName   State
@@ -775,7 +775,7 @@ printed in an extra column. This flag is usable only with 
the default
 
 **Example 2:**
 
-.. code-block::
+::
 
$ virsh list --title
  IdNameState  Title
@@ -790,7 +790,7 @@ freecell
 
 **Syntax:**
 
-.. code-block::
+::
 
freecell [{ [--cellno] cellno | --all }]
 
@@ -809,7 +809,7 @@ freepages
 
 **Syntax:**
 
-.. code-block::
+::
 
freepages [{ [--cellno] cellno [--pagesize] pagesize | --all }]
 
@@ -824,7 +824,7 @@ a

Re: [libvirt PATCH 0/4] vz: use g_new0 (glib chronicles)

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 12:23:55AM +0200, Ján Tomko wrote:
> Ján Tomko (4):
>   vz: separate if conditions in vzEatCookie
>   vz: vzEatCookie: use distinct 'tmp' variables
>   vz: vzEatCookie: separate allocation
>   vz: use g_new0 instead of VIR_ALLOC
> 
>  src/vz/vz_driver.c | 49 ++
>  src/vz/vz_sdk.c| 46 +--
>  src/vz/vz_utils.c  |  3 +--
>  3 files changed, 38 insertions(+), 60 deletions(-)
> 

Reviewed-by: Erik Skultety 



[libvirt PATCHv2] qemu: process: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
Oops, forgot to squash some changes in.

 src/qemu/qemu_process.c | 56 +
 1 file changed, 18 insertions(+), 38 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 93cbb37986..cb378a86aa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -302,8 +302,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon,
 goto cleanup;
 }
 
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_MONITOR_EOF;
 processEvent->vm = virObjectRef(vm);
@@ -832,8 +831,7 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon G_GNUC_UNUSED,
 
 if (vm->def->watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) {
 struct qemuProcessEvent *processEvent;
-if (VIR_ALLOC(processEvent) < 0)
-;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_WATCHDOG;
 processEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP;
@@ -965,8 +963,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon G_GNUC_UNUSED,
 virDomainObjBroadcast(vm);
 } else {
 /* there is no waiting SYNC API, dispatch the update to a thread */
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_BLOCK_JOB;
 data = g_strdup(diskAlias);
@@ -1030,8 +1027,7 @@ qemuProcessHandleJobStatusChange(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 virDomainObjBroadcast(vm);
 } else {
 VIR_DEBUG("job '%s' handled by event thread", jobname);
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE;
 processEvent->vm = virObjectRef(vm);
@@ -1074,20 +1070,17 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 virDomainEventGraphicsSubjectPtr subject = NULL;
 size_t i;
 
-if (VIR_ALLOC(localAddr) < 0)
-goto error;
+localAddr = g_new0(virDomainEventGraphicsAddress, 1);
 localAddr->family = localFamily;
 localAddr->service = g_strdup(localService);
 localAddr->node = g_strdup(localNode);
 
-if (VIR_ALLOC(remoteAddr) < 0)
-goto error;
+remoteAddr = g_new0(virDomainEventGraphicsAddress, 1);
 remoteAddr->family = remoteFamily;
 remoteAddr->service = g_strdup(remoteService);
 remoteAddr->node = g_strdup(remoteNode);
 
-if (VIR_ALLOC(subject) < 0)
-goto error;
+subject = g_new0(virDomainEventGraphicsSubject, 1);
 if (x509dname) {
 if (VIR_REALLOC_N(subject->identities, subject->nidentity+1) < 0)
 goto error;
@@ -1329,8 +1322,7 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 struct qemuProcessEvent *processEvent;
 
 virObjectLock(vm);
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_GUESTPANIC;
 processEvent->action = vm->def->onCrash;
@@ -1345,7 +1337,6 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 qemuProcessEventFree(processEvent);
 }
 
- cleanup:
 virObjectUnlock(vm);
 
 return 0;
@@ -1371,8 +1362,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon 
G_GNUC_UNUSED,
   QEMU_DOMAIN_UNPLUGGING_DEVICE_STATUS_OK))
 goto cleanup;
 
-if (VIR_ALLOC(processEvent) < 0)
-goto error;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_DEVICE_DELETED;
 data = g_strdup(devAlias);
@@ -1552,8 +1542,7 @@ qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 VIR_DEBUG("Device %s RX Filter changed in domain %p %s",
   devAlias, vm, vm->def->name);
 
-if (VIR_ALLOC(processEvent) < 0)
-goto error;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED;
 data = g_strdup(devAlias);
@@ -1590,8 +1579,7 @@ qemuProcessHandleSerialChanged(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 VIR_DEBUG("Serial port %s state changed to '%d' in domain %p %s",
   devAlias, connected, vm, vm->def->name);
 
-if (VIR_ALLOC(processEvent) < 0)
-goto error;
+processEvent = g_new0(struct qemuProcessEvent, 1);
 
 processEvent->eventType = QEMU_PROCESS_EVENT_SERIAL_CHANGED;
 data = g_strdup(devAlias);
@@ -1800,8 +1788,7 @@ qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr 
mon G_GNUC_UNUSED,
 priv = vm->privateData;
 priv->prDaemonRunning = false;
 
-if (VIR_ALLOC(processEvent) < 0)
-goto cleanup;
+processEve

Re: [libvirt PATCH 4/4] vbox: use g_new0 instead of VIR_ALLOC

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Erik Skultety wrote:

On Mon, Oct 05, 2020 at 12:22:26AM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---

...



 for (i = 0; i < sharedFolders.count; i++) {
 ISharedFolder *sharedFolder = sharedFolders.items[i];
@@ -3798,10 +3789,8 @@ vboxDumpAudio(virDomainDefPtr def, vboxDriverPtr data 
G_GNUC_UNUSED,
 PRUint32 audioController = AudioControllerType_AC97;

 def->nsounds = 1;
-if (VIR_ALLOC_N(def->sounds, def->nsounds) < 0)
-return;
-if (VIR_ALLOC(def->sounds[0]) < 0)
-return;
+def->sounds = g_new0(virDomainSoundDefPtr, def->nsounds);


just use 1 for the counter


I can do that.


and drop the nsounds attribute initialization
completely.


def->nsounds needs to be set to 1, otherwise virDomainDefFormat would
not even look at def->sounds.

Jano



Reviewed-by: Erik Skultety 


signature.asc
Description: PGP signature


Re: [libvirt PATCH 5/8] esx: esxConnectOpen: use allocated buffer

2020-10-05 Thread Pino Toscano
On Monday, 5 October 2020 00:21:42 CEST Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/esx/esx_driver.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index e82e5ed835..0798493296 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>  virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
>  esxPrivate *priv = NULL;
>  char *potentialVCenterIPAddress = NULL;
> -char vCenterIPAddress[NI_MAXHOST] = "";
> +g_autofree char *vCenterIPAddress = g_new0(char, NI_MAXHOST);

Hmm, this is not the only char[NI_MAXHOST] in this file, so I'm
surprised only this one triggered the stack limit check.

The NI_MAXHOST-limited buffer seems to be due to
esxUtil_ResolveHostname(), which forces it due to its interface.
I'll rewrite it to return a new string instead; consider it as a NACK
for this patch.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.


[libvirt PATCH 1/2] esx: call freeaddrinfo earlier in esxUtil_ResolveHostname

2020-10-05 Thread Pino Toscano
Call freeaddrinfo() as soon as @result is not needed anymore, i.e. right
after getnameinfo(); this avoids calling freeaddrinfo() in two branches.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_util.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 9100873326..555158f953 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -310,17 +310,15 @@ esxUtil_ResolveHostname(const char *hostname,
 
 errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,
   ipAddress_length, NULL, 0, NI_NUMERICHOST);
+freeaddrinfo(result);
 
 if (errcode != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Formatting IP address for host '%s' failed: %s"), 
hostname,
gai_strerror(errcode));
-freeaddrinfo(result);
 return -1;
 }
 
-freeaddrinfo(result);
-
 return 0;
 }
 
-- 
2.26.2



[libvirt PATCH 2/2] esx: switch esxUtil_ResolveHostname to return a new string

2020-10-05 Thread Pino Toscano
Change the interface of esxUtil_ResolveHostname() to return a newly
allocated string with the result address, instead of forcing the callers
to provide a buffer and its size. This way we can simply (auto)free the
string, and make the function stacks smaller.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_driver.c | 20 +++-
 src/esx/esx_util.c   | 11 +++
 src/esx/esx_util.h   |  3 +--
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index e82e5ed835..a17bf58a51 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -602,7 +602,7 @@ esxConnectToHost(esxPrivate *priv,
  char **vCenterIPAddress)
 {
 int result = -1;
-char ipAddress[NI_MAXHOST] = "";
+g_autofree char *ipAddress = NULL;
 char *username = NULL;
 char *password = NULL;
 char *url = NULL;
@@ -615,7 +615,7 @@ esxConnectToHost(esxPrivate *priv,
 
 ESX_VI_CHECK_ARG_LIST(vCenterIPAddress);
 
-if (esxUtil_ResolveHostname(conn->uri->server, ipAddress, NI_MAXHOST) < 0)
+if (esxUtil_ResolveHostname(conn->uri->server, &ipAddress) < 0)
 return -1;
 
 if (conn->uri->user) {
@@ -692,7 +692,7 @@ esxConnectToVCenter(esxPrivate *priv,
 const char *hostSystemIPAddress)
 {
 int result = -1;
-char ipAddress[NI_MAXHOST] = "";
+g_autofree char *ipAddress = NULL;
 char *username = NULL;
 char *password = NULL;
 char *url = NULL;
@@ -704,7 +704,7 @@ esxConnectToVCenter(esxPrivate *priv,
 return -1;
 }
 
-if (esxUtil_ResolveHostname(hostname, ipAddress, NI_MAXHOST) < 0)
+if (esxUtil_ResolveHostname(hostname, &ipAddress) < 0)
 return -1;
 
 if (conn->uri->user) {
@@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
 virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
 esxPrivate *priv = NULL;
 char *potentialVCenterIPAddress = NULL;
-char vCenterIPAddress[NI_MAXHOST] = "";
+g_autofree char *vCenterIPAddress = NULL;
 
 virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
 
@@ -875,16 +875,10 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
 goto cleanup;
 }
 
-if (virStrcpyStatic(vCenterIPAddress,
-potentialVCenterIPAddress) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("vCenter IP address %s too big for 
destination"),
-   potentialVCenterIPAddress);
-goto cleanup;
-}
+vCenterIPAddress = g_strdup(potentialVCenterIPAddress);
 } else {
 if (esxUtil_ResolveHostname(priv->parsedUri->vCenter,
-vCenterIPAddress, NI_MAXHOST) < 0) 
{
+&vCenterIPAddress) < 0) {
 goto cleanup;
 }
 
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 555158f953..d9e7641d67 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -278,12 +278,12 @@ esxUtil_ParseDatastorePath(const char *datastorePath, 
char **datastoreName,
 
 
 int
-esxUtil_ResolveHostname(const char *hostname,
-char *ipAddress, size_t ipAddress_length)
+esxUtil_ResolveHostname(const char *hostname, char **ipAddress)
 {
 struct addrinfo hints;
 struct addrinfo *result = NULL;
 int errcode;
+g_autofree char *address = NULL;
 
 memset(&hints, 0, sizeof(hints));
 
@@ -308,8 +308,9 @@ esxUtil_ResolveHostname(const char *hostname,
 return -1;
 }
 
-errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,
-  ipAddress_length, NULL, 0, NI_NUMERICHOST);
+address = g_new0(char, NI_MAXHOST);
+errcode = getnameinfo(result->ai_addr, result->ai_addrlen, address,
+  NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
 freeaddrinfo(result);
 
 if (errcode != 0) {
@@ -319,6 +320,8 @@ esxUtil_ResolveHostname(const char *hostname,
 return -1;
 }
 
+*ipAddress = g_strdup(address);
+
 return 0;
 }
 
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
index 97b6d82a2b..9bfbff1d42 100644
--- a/src/esx/esx_util.h
+++ b/src/esx/esx_util.h
@@ -55,8 +55,7 @@ int esxUtil_ParseVirtualMachineIDString(const char 
*id_string, int *id);
 int esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName,
char **directoryName, char 
**directoryAndFileName);
 
-int esxUtil_ResolveHostname(const char *hostname,
-char *ipAddress, size_t ipAddress_length);
+int esxUtil_ResolveHostname(const char *hostname, char **ipAddress);
 
 int esxUtil_ReformatUuid(const char *input, char *output);
 
-- 
2.26.2



Re: [libvirt PATCH 9/9] tests: Add tests for unknown elements and attributes in cpu defintion

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Tim Wiederhake wrote:

On Wed, 2020-09-30 at 14:11 +0200, Peter Krempa wrote:

On Wed, Sep 30, 2020 at 13:54:58 +0200, Tim Wiederhake wrote:
> On Tue, 2020-09-29 at 11:22 +0200, Peter Krempa wrote:
> > On Mon, Sep 21, 2020 at 15:07:32 +0200, Tim Wiederhake wrote:
> > > Signed-off-by: Tim Wiederhake 
> > > ---
> >
> > [...]
> >
> > > diff --git a/tests/cputestdata/x86_64-bogus-element.xml
> > > b/tests/cputestdata/x86_64-bogus-element.xml
> > > new file mode 100644
> > > index 00..79f98bad18
> > > --- /dev/null
> > > +++ b/tests/cputestdata/x86_64-bogus-element.xml
> > > @@ -0,0 +1,3 @@
> > > +
> > > +  
> > > +
> >
> > I'm not persuaded that there's value in such test.
> >
>
> This was my approach to adding negative tests to the "virsh
> [hypervisor-]cpu-compare --validate" feature to make sure that
> schema
> violations are actually caught. If this is not the correct place to
> do
> so, I would be grateful for a pointer in the right direction.

I'd find way more useful to actually test that all the XMLs we use in
the tests are validated, which does not seem to happen in this
series.

This series adds schema validation only for a very synthetic negative
case which IMO doesn't make sense.

Additionally we do have 'virschematest', which is meant to validate
all
XML documents in the tests. Based on the fact that you've added a
invalid XML example but it's name doesn't end in '-invalid.xml',
which
is the marker for the 'virschematest' worker to do negative
validation,
means that the cpu files are not validated.

Rather than adding a synthetic case, wire up virschematest which will
make sure that we don't have bogus XMLs in the test suite.


Thanks, I was unaware of this test. One problem I see is that
tests/cputestdata contains xml files with different root elements:

$ for i in tests/cputestdata/*.xml ; do xpath -q -e "name(/*)" $i ;
done | sort | uniq -c
   273 cpu
   157 cpudata
16 cpuTest

Adding 'DO_TEST_DIR("cpu.rng", "cputestdata");' to virschematest.c
therefore fails for 157 + 16 files. I do not believe that we would want
to individually list 273 files in virschematest.c and reworking the
directory structure appears undesirable.


If by undesirable you mean you don't want to touch it with a 3.048 m
pole, I understand.

But it's not trivial to figure out from cputest.c or the filenames
which files belong to which tests, so separating them to different
directories might help readability.

The 'cpudata' files should have -cpuid- in their name, so separating
them to a separate directory shouldn't be so difficult.

And  is used as a wrapper for multiple  nodes for
baseline tests.

Jano


Do you have any suggestions on
how to go forward on this?

Thanks,
Tim




signature.asc
Description: PGP signature


[PATCH v2 4/7] virbitmaptest: Turn 'TEST_MAP' macro into a helper function

2020-10-05 Thread Peter Krempa
The function will also be reusable in other places of the code by making
the size check optional. For now only test12* is refactored since it
used TEST_MAP directly.

Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 63 +--
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index dc23431645..c16be62e23 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -26,6 +26,31 @@

 #include "virbitmap.h"

+
+static int
+checkBitmap(virBitmapPtr map,
+const char *expect,
+ssize_t expectedSize)
+{
+g_autofree char *actual = virBitmapFormat(map);
+
+if (expectedSize != -1 &&
+virBitmapSize(map) != expectedSize) {
+fprintf(stderr, "\n expected bitmap size: '%zd' actual size: "
+"'%zu'\n", expectedSize, virBitmapSize(map));
+return -1;
+}
+
+if (STRNEQ_NULLABLE(expect, actual)) {
+fprintf(stderr, "\n expected bitmap contents '%s' actual contents "\
+"'%s'\n", NULLSTR(expect), NULLSTR(actual));
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 test1(const void *data G_GNUC_UNUSED)
 {
@@ -630,25 +655,6 @@ test11(const void *opaque)
 return ret;
 }

-#define TEST_MAP(sz, expect) \
-do { \
-char *actual; \
-if (virBitmapSize(map) != sz) { \
-fprintf(stderr, "\n expected bitmap size: '%d' actual size: " \
-"'%zu'\n", sz, virBitmapSize(map)); \
-goto cleanup; \
-} \
- \
-actual = virBitmapFormat(map); \
- \
-if (STRNEQ_NULLABLE(expect, actual)) { \
-fprintf(stderr, "\n expected bitmap contents '%s' actual contents 
"\
-"'%s'\n", NULLSTR(expect), NULLSTR(actual)); \
-VIR_FREE(actual); \
-goto cleanup; \
-} \
-VIR_FREE(actual); \
-} while (0)

 /* test self-expanding bitmap APIs */
 static int
@@ -657,17 +663,20 @@ test12a(const void *opaque G_GNUC_UNUSED)
 virBitmapPtr map = virBitmapNewEmpty();
 int ret = -1;

-TEST_MAP(0, "");
+if (checkBitmap(map, "", 0) < 0)
+goto cleanup;

 if (virBitmapSetBitExpand(map, 128) < 0)
 goto cleanup;

-TEST_MAP(129, "128");
+if (checkBitmap(map, "128", 129) < 0)
+goto cleanup;

 if (virBitmapClearBitExpand(map, 150) < 0)
 goto cleanup;

-TEST_MAP(151, "128");
+if (checkBitmap(map, "128", 151) < 0)
+goto cleanup;

 ret = 0;

@@ -686,13 +695,16 @@ test12b(const void *opaque G_GNUC_UNUSED)
 if (!(map = virBitmapParseUnlimited("34,1023")))
 goto cleanup;

-TEST_MAP(1024, "34,1023");
+if (checkBitmap(map, "34,1023", 1024) < 0)
+goto cleanup;

 virBitmapShrink(map, 35);
-TEST_MAP(35, "34");
+if (checkBitmap(map, "34", 35) < 0)
+goto cleanup;

 virBitmapShrink(map, 34);
-TEST_MAP(34, "");
+if (checkBitmap(map, "", 34) < 0)
+goto cleanup;

 ret = 0;

@@ -729,7 +741,6 @@ test13(const void *opaque G_GNUC_UNUSED)
 return 0;
 }

-#undef TEST_MAP

 static int
 test14(const void *opaque)
-- 
2.26.2



[PATCH v2 5/7] virbitmaptest: Refactor checks in 'test6'

2020-10-05 Thread Peter Krempa
The 'checkBitmap' helper uses 'virBitmapFormat' internally and also
reports better errors. Use it instead of the open-coded checks.

Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 44 ++-
 1 file changed, 6 insertions(+), 38 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index c16be62e23..c59eb49265 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -407,7 +407,6 @@ static int
 test6(const void *v G_GNUC_UNUSED)
 {
 virBitmapPtr bitmap = NULL;
-char *str = NULL;
 int size = 64;
 int ret = -1;

@@ -415,73 +414,42 @@ test6(const void *v G_GNUC_UNUSED)
 if (!bitmap)
 goto error;

-str = virBitmapFormat(bitmap);
-if (!str)
+if (checkBitmap(bitmap, "", -1) < 0)
 goto error;

-if (STRNEQ(str, ""))
-goto error;
-
-VIR_FREE(str);
-
 ignore_value(virBitmapSetBit(bitmap, 0));
-str = virBitmapFormat(bitmap);
-if (!str)
-goto error;

-if (STRNEQ(str, "0"))
+if (checkBitmap(bitmap, "0", -1) < 0)
 goto error;

-VIR_FREE(str);
-
 ignore_value(virBitmapSetBit(bitmap, 4));
 ignore_value(virBitmapSetBit(bitmap, 5));
-str = virBitmapFormat(bitmap);
-if (!str)
-goto error;

-if (STRNEQ(str, "0,4-5"))
+if (checkBitmap(bitmap, "0,4-5", -1) < 0)
 goto error;

-VIR_FREE(str);
-
 ignore_value(virBitmapSetBit(bitmap, 6));
-str = virBitmapFormat(bitmap);
-if (!str)
-goto error;

-if (STRNEQ(str, "0,4-6"))
+if (checkBitmap(bitmap, "0,4-6", -1) < 0)
 goto error;

-VIR_FREE(str);
-
 ignore_value(virBitmapSetBit(bitmap, 13));
 ignore_value(virBitmapSetBit(bitmap, 14));
 ignore_value(virBitmapSetBit(bitmap, 15));
 ignore_value(virBitmapSetBit(bitmap, 16));
-str = virBitmapFormat(bitmap);
-if (!str)
-goto error;

-if (STRNEQ(str, "0,4-6,13-16"))
+if (checkBitmap(bitmap, "0,4-6,13-16", -1) < 0)
 goto error;

-VIR_FREE(str);
-
 ignore_value(virBitmapSetBit(bitmap, 62));
 ignore_value(virBitmapSetBit(bitmap, 63));
-str = virBitmapFormat(bitmap);
-if (!str)
-goto error;

-if (STRNEQ(str, "0,4-6,13-16,62-63"))
+if (checkBitmap(bitmap, "0,4-6,13-16,62-63", -1) < 0)
 goto error;

-
 ret = 0;
  error:
 virBitmapFree(bitmap);
-VIR_FREE(str);
 return ret;
 }

-- 
2.26.2



[PATCH v2 0/7] virbitmaptest: Refactor and modernize

2020-10-05 Thread Peter Krempa
This was split out from my series refactoring the virbitmap util module
to address review feedback.

Patches 1-5 are new, patches 6-7 were already part of the refactor but
needed to be modified to fit on top of the changes needed to comply with
review requirements.

Peter Krempa (7):
  virbitmaptest: Split up test12
  virbitmaptest: Split up test4
  virbitmaptest: Use separate output strings in 'test5'
  virbitmaptest: Turn 'TEST_MAP' macro into a helper function
  virbitmaptest: Refactor checks in 'test6'
  virbitmaptest: Use g_auto(free) for cleanup
  virbitmaptest: Remove unnecessary error/cleanup labels

 tests/virbitmaptest.c | 512 ++
 1 file changed, 224 insertions(+), 288 deletions(-)

-- 
2.26.2



[PATCH v2 7/7] virbitmaptest: Remove unnecessary error/cleanup labels

2020-10-05 Thread Peter Krempa
Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 270 +-
 1 file changed, 107 insertions(+), 163 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index fd49ed4c55..3559e61be7 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -58,32 +58,28 @@ test1(const void *data G_GNUC_UNUSED)
 int size;
 int bit;
 bool result;
-int ret = -1;

 size = 1024;
 bit = 100;
 if (!(bitmap = virBitmapNew(size)))
-goto error;
+return -1;

 if (virBitmapSetBit(bitmap, bit) < 0)
-goto error;
+return -1;

 if (virBitmapGetBit(bitmap, bit, &result) < 0)
-goto error;
+return -1;

 if (!result)
-goto error;
+return -1;

 if (virBitmapGetBit(bitmap, bit + 1, &result) < 0)
-goto error;
+return -1;

 if (result)
-goto error;
-
-ret = 0;
+return -1;

- error:
-return ret;
+return 0;
 }

 static int
@@ -111,85 +107,78 @@ test2(const void *data G_GNUC_UNUSED)
 const char *bitsString1 = "1-32,50,88-99,1021-1023";
 g_autofree char *bitsString2 = NULL;
 g_autoptr(virBitmap) bitmap = NULL;
-int ret = -1;
 int size = 1025;

 if (virBitmapParse(bitsString1, &bitmap, size) < 0)
-goto error;
+return -1;

 if (testBit(bitmap, 1, 32, true) < 0)
-goto error;
+return -1;
 if (testBit(bitmap, 50, 50, true) < 0)
-goto error;
+return -1;
 if (testBit(bitmap, 88, 99, true) < 0)
-goto error;
+return -1;
 if (testBit(bitmap, 1021, 1023, true) < 0)
-goto error;
+return -1;

 if (testBit(bitmap, 0, 0, false) < 0)
-goto error;
+return -1;
 if (testBit(bitmap, 33, 49, false) < 0)
-goto error;
+return -1;
 if (testBit(bitmap, 51, 87, false) < 0)
-goto error;
+return -1;
 if (testBit(bitmap, 100, 1020, false) < 0)
-goto error;
+return -1;

 if (virBitmapCountBits(bitmap) != 48)
-goto error;
+return -1;

 if (!(bitsString2 = virBitmapFormat(bitmap)))
-goto error;
+return -1;
 if (strcmp(bitsString1, bitsString2))
-goto error;
+return -1;

 virBitmapSetAll(bitmap);
 if (testBit(bitmap, 0, size - 1, true) < 0)
-goto error;
+return -1;
 if (virBitmapCountBits(bitmap) != size)
-goto error;
+return -1;

 if (!virBitmapIsAllSet(bitmap))
-goto error;
+return -1;

 virBitmapClearAll(bitmap);
 if (!virBitmapIsAllClear(bitmap))
-goto error;
+return -1;
 if (testBit(bitmap, 0, size - 1, false) < 0)
-goto error;
+return -1;
 if (virBitmapCountBits(bitmap) != 0)
-goto error;
-
-ret = 0;
+return -1;

- error:
-return ret;
+return 0;
 }

 static int
 test3(const void *data G_GNUC_UNUSED)
 {
 g_autoptr(virBitmap) bitmap = NULL;
-int ret = -1;
 int size = 5;
 size_t i;

 if ((bitmap = virBitmapNew(size)) == NULL)
-goto error;
+return -1;

 for (i = 0; i < size; i++)
 ignore_value(virBitmapSetBit(bitmap, i));

 if (!virBitmapIsAllSet(bitmap))
-goto error;
+return -1;

 virBitmapClearAll(bitmap);
 if (!virBitmapIsAllClear(bitmap))
-goto error;
-ret = 0;
+return -1;

- error:
-return ret;
+return 0;
 }

 /* test for virBitmapNextSetBit, virBitmapLastSetBit, virBitmapNextClearBit */
@@ -203,18 +192,15 @@ test4a(const void *data G_GNUC_UNUSED)
 bitmap = virBitmapNewEmpty();

 if (virBitmapNextSetBit(bitmap, -1) != -1)
-goto error;
+return -1;

 if (virBitmapLastSetBit(bitmap) != -1)
-goto error;
+return -1;

 if (virBitmapNextClearBit(bitmap, -1) != -1)
-goto error;
+return -1;

 return 0;
-
- error:
-return -1;
 }


@@ -229,28 +215,25 @@ test4b(const void *data G_GNUC_UNUSED)

 bitmap = virBitmapNew(size);
 if (!bitmap)
-goto error;
+return -1;

 if (virBitmapNextSetBit(bitmap, -1) != -1)
-goto error;
+return -1;

 if (virBitmapLastSetBit(bitmap) != -1)
-goto error;
+return -1;

 for (i = 0; i < size; i++) {
 if (virBitmapNextClearBit(bitmap, i - 1) != i)
-goto error;
+return -1;
 }
 if (virBitmapNextClearBit(bitmap, i) != -1)
-goto error;
+return -1;

 if (!virBitmapIsAllClear(bitmap))
-goto error;
+return -1;

 return 0;
-
- error:
-return -1;
 }


@@ -271,14 +254,14 @@ test4c(const void *data G_GNUC_UNUSED)
 ssize_t i, j;

 if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size)
-goto error;
+return -1;

 /* 2. partial set */

 if (virBitmapParse(bitsString, &bitmap, size) < 0)
-goto 

[PATCH v2 1/7] virbitmaptest: Split up test12

2020-10-05 Thread Peter Krempa
'test12' was testing two distinct operations on two instances of a
bitmap. Split it up into 'test12a' and 'test12b' so that the 'bitmap'
variable is not reused.

Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 687b5f87af..32187ebec4 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -629,7 +629,7 @@ test11(const void *opaque)

 /* test self-expanding bitmap APIs */
 static int
-test12(const void *opaque G_GNUC_UNUSED)
+test12a(const void *opaque G_GNUC_UNUSED)
 {
 virBitmapPtr map = virBitmapNewEmpty();
 int ret = -1;
@@ -646,7 +646,20 @@ test12(const void *opaque G_GNUC_UNUSED)

 TEST_MAP(151, "128");

+ret = 0;
+
+ cleanup:
 virBitmapFree(map);
+return ret;
+}
+
+
+static int
+test12b(const void *opaque G_GNUC_UNUSED)
+{
+virBitmapPtr map = virBitmapNewEmpty();
+int ret = -1;
+
 if (!(map = virBitmapParseUnlimited("34,1023")))
 goto cleanup;

@@ -829,7 +842,9 @@ mymain(void)
 TESTBINARYOP("0-3", "0,^0", "0,^0", test11);
 TESTBINARYOP("0,2", "1,3", "0,^0", test11);

-if (virTestRun("test12", test12, NULL) < 0)
+if (virTestRun("test12a", test12a, NULL) < 0)
+ret = -1;
+if (virTestRun("test12b", test12b, NULL) < 0)
 ret = -1;
 if (virTestRun("test13", test13, NULL) < 0)
 ret = -1;
-- 
2.26.2



[PATCH v2 2/7] virbitmaptest: Split up test4

2020-10-05 Thread Peter Krempa
'test4' was testing three distinct operations on separate instances of a
bitmap. Split it up into 'test4a', 'test4b' and 'test4c' so that the
'bitmap' variable is not reused.

Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 64 +++
 1 file changed, 46 insertions(+), 18 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 32187ebec4..98ac06c406 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -173,23 +173,9 @@ test3(const void *data G_GNUC_UNUSED)

 /* test for virBitmapNextSetBit, virBitmapLastSetBit, virBitmapNextClearBit */
 static int
-test4(const void *data G_GNUC_UNUSED)
+test4a(const void *data G_GNUC_UNUSED)
 {
-const char *bitsString = "0, 2-4, 6-10, 12, 14-18, 20, 22, 25";
-int size = 40;
-int bitsPos[] = {
-0,  2,  3,  4,  6,  7,  8,  9, 10, 12,
-14, 15, 16, 17, 18, 20, 22, 25
-};
-int bitsPosInv[] = {
-1, 5, 11, 13, 19, 21, 23, 24, 26, 27,
-28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39
-};
 virBitmapPtr bitmap = NULL;
-ssize_t i, j;
-
-if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size)
-goto error;

 /* 0. empty set */

@@ -205,7 +191,20 @@ test4(const void *data G_GNUC_UNUSED)
 goto error;

 virBitmapFree(bitmap);
-bitmap = NULL;
+return 0;
+
+ error:
+virBitmapFree(bitmap);
+return -1;
+}
+
+
+static int
+test4b(const void *data G_GNUC_UNUSED)
+{
+virBitmapPtr bitmap = NULL;
+int size = 40;
+size_t i;

 /* 1. zero set */

@@ -230,7 +229,32 @@ test4(const void *data G_GNUC_UNUSED)
 goto error;

 virBitmapFree(bitmap);
-bitmap = NULL;
+return 0;
+
+ error:
+virBitmapFree(bitmap);
+return -1;
+}
+
+
+static int
+test4c(const void *data G_GNUC_UNUSED)
+{
+const char *bitsString = "0, 2-4, 6-10, 12, 14-18, 20, 22, 25";
+int size = 40;
+int bitsPos[] = {
+0,  2,  3,  4,  6,  7,  8,  9, 10, 12,
+14, 15, 16, 17, 18, 20, 22, 25
+};
+int bitsPosInv[] = {
+1, 5, 11, 13, 19, 21, 23, 24, 26, 27,
+28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39
+};
+virBitmapPtr bitmap = NULL;
+ssize_t i, j;
+
+if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size)
+goto error;

 /* 2. partial set */

@@ -818,7 +842,11 @@ mymain(void)
 ret = -1;
 if (virTestRun("test3", test3, NULL) < 0)
 ret = -1;
-if (virTestRun("test4", test4, NULL) < 0)
+if (virTestRun("test4a", test4a, NULL) < 0)
+ret = -1;
+if (virTestRun("test4b", test4b, NULL) < 0)
+ret = -1;
+if (virTestRun("test4c", test4c, NULL) < 0)
 ret = -1;
 if (virTestRun("test5", test5, NULL) < 0)
 ret = -1;
-- 
2.26.2



[PATCH v2 3/7] virbitmaptest: Use separate output strings in 'test5'

2020-10-05 Thread Peter Krempa
The test validates two outputs. Don't reuse 'str' for both.

Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 98ac06c406..dc23431645 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -329,7 +329,8 @@ test5(const void *v G_GNUC_UNUSED)
 size_t i;
 ssize_t j;
 int ret = -1;
-char *str = NULL;
+g_autofree char *actual1 = NULL;
+g_autofree char *actual2 = NULL;

 bitmap = virBitmapNewData(data, sizeof(data));
 if (!bitmap)
@@ -359,19 +360,17 @@ test5(const void *v G_GNUC_UNUSED)
 data2[4] != 0x04)
 goto error;

-if (!(str = virBitmapDataFormat(data, sizeof(data
+if (!(actual1 = virBitmapDataFormat(data, sizeof(data
 goto error;
-if (STRNEQ(str, "0,9,34"))
+if (STRNEQ(actual1, "0,9,34"))
 goto error;
-VIR_FREE(str);
-if (!(str = virBitmapDataFormat(data2, len2)))
+if (!(actual2 = virBitmapDataFormat(data2, len2)))
 goto error;
-if (STRNEQ(str, "0,2,9,15,34"))
+if (STRNEQ(actual2, "0,2,9,15,34"))
 goto error;

 ret = 0;
  error:
-VIR_FREE(str);
 virBitmapFree(bitmap);
 VIR_FREE(data2);
 return ret;
-- 
2.26.2



[PATCH v2 6/7] virbitmaptest: Use g_auto(free) for cleanup

2020-10-05 Thread Peter Krempa
Signed-off-by: Peter Krempa 
---
 tests/virbitmaptest.c | 79 ++-
 1 file changed, 25 insertions(+), 54 deletions(-)

diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index c59eb49265..fd49ed4c55 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -54,7 +54,7 @@ checkBitmap(virBitmapPtr map,
 static int
 test1(const void *data G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap;
+g_autoptr(virBitmap) bitmap = NULL;
 int size;
 int bit;
 bool result;
@@ -83,7 +83,6 @@ test1(const void *data G_GNUC_UNUSED)
 ret = 0;

  error:
-virBitmapFree(bitmap);
 return ret;
 }

@@ -110,8 +109,8 @@ static int
 test2(const void *data G_GNUC_UNUSED)
 {
 const char *bitsString1 = "1-32,50,88-99,1021-1023";
-char *bitsString2 = NULL;
-virBitmapPtr bitmap = NULL;
+g_autofree char *bitsString2 = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
 int ret = -1;
 int size = 1025;

@@ -164,15 +163,13 @@ test2(const void *data G_GNUC_UNUSED)
 ret = 0;

  error:
-virBitmapFree(bitmap);
-VIR_FREE(bitsString2);
 return ret;
 }

 static int
 test3(const void *data G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
 int ret = -1;
 int size = 5;
 size_t i;
@@ -192,7 +189,6 @@ test3(const void *data G_GNUC_UNUSED)
 ret = 0;

  error:
-virBitmapFree(bitmap);
 return ret;
 }

@@ -200,7 +196,7 @@ test3(const void *data G_GNUC_UNUSED)
 static int
 test4a(const void *data G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;

 /* 0. empty set */

@@ -215,11 +211,9 @@ test4a(const void *data G_GNUC_UNUSED)
 if (virBitmapNextClearBit(bitmap, -1) != -1)
 goto error;

-virBitmapFree(bitmap);
 return 0;

  error:
-virBitmapFree(bitmap);
 return -1;
 }

@@ -227,7 +221,7 @@ test4a(const void *data G_GNUC_UNUSED)
 static int
 test4b(const void *data G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
 int size = 40;
 size_t i;

@@ -253,11 +247,9 @@ test4b(const void *data G_GNUC_UNUSED)
 if (!virBitmapIsAllClear(bitmap))
 goto error;

-virBitmapFree(bitmap);
 return 0;

  error:
-virBitmapFree(bitmap);
 return -1;
 }

@@ -275,7 +267,7 @@ test4c(const void *data G_GNUC_UNUSED)
 1, 5, 11, 13, 19, 21, 23, 24, 26, 27,
 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39
 };
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
 ssize_t i, j;

 if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size)
@@ -334,11 +326,9 @@ test4c(const void *data G_GNUC_UNUSED)
 if (virBitmapNextClearBit(bitmap, -1) != -1)
 goto error;

-virBitmapFree(bitmap);
 return 0;

  error:
-virBitmapFree(bitmap);
 return -1;
 }

@@ -347,10 +337,10 @@ static int
 test5(const void *v G_GNUC_UNUSED)
 {
 char data[] = {0x01, 0x02, 0x00, 0x00, 0x04};
-unsigned char *data2 = NULL;
+g_autofree unsigned char *data2 = NULL;
 int len2;
 int bits[] = {0, 9, 34};
-virBitmapPtr bitmap;
+g_autoptr(virBitmap) bitmap = NULL;
 size_t i;
 ssize_t j;
 int ret = -1;
@@ -396,8 +386,6 @@ test5(const void *v G_GNUC_UNUSED)

 ret = 0;
  error:
-virBitmapFree(bitmap);
-VIR_FREE(data2);
 return ret;
 }

@@ -406,7 +394,7 @@ test5(const void *v G_GNUC_UNUSED)
 static int
 test6(const void *v G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
 int size = 64;
 int ret = -1;

@@ -449,14 +437,12 @@ test6(const void *v G_GNUC_UNUSED)

 ret = 0;
  error:
-virBitmapFree(bitmap);
 return ret;
 }

 static int
 test7(const void *v G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap;
 size_t i;
 size_t maxBit[] = {
 1, 8, 31, 32, 63, 64, 95, 96, 127, 128, 159, 160
@@ -464,7 +450,7 @@ test7(const void *v G_GNUC_UNUSED)
 size_t nmaxBit = 12;

 for (i = 0; i < nmaxBit; i++) {
-bitmap = virBitmapNew(maxBit[i]);
+g_autoptr(virBitmap) bitmap = virBitmapNew(maxBit[i]);
 if (!bitmap)
 goto error;

@@ -482,21 +468,18 @@ test7(const void *v G_GNUC_UNUSED)
 virBitmapClearAll(bitmap);
 if (!virBitmapIsAllClear(bitmap))
 goto error;
-
-virBitmapFree(bitmap);
 }

 return 0;

  error:
-virBitmapFree(bitmap);
 return -1;
 }

 static int
 test8(const void *v G_GNUC_UNUSED)
 {
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
 char data[108] = {0x00,};
 int ret = -1;

@@ -515,7 +498,6 @@ test8(const void *v G_GNUC_UNUSED)

 ret = 0;
  cleanup:
-virBitmapFree(bitmap);
 return ret;
 }

@@ -525,7 +507,7 @@ static int
 test9(const void *opaque G_GNUC_UNUSED)
 {
 int ret = -1;
-virBitmapPtr bitmap = NULL;
+g_autoptr(virBitmap) bitmap = NULL;

 if (virBitmapParse("1000

Re: [libvirt PATCH 4/4] vbox: use g_new0 instead of VIR_ALLOC

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 01:14:51PM +0200, Ján Tomko wrote:
> On a Monday in 2020, Erik Skultety wrote:
> > On Mon, Oct 05, 2020 at 12:22:26AM +0200, Ján Tomko wrote:
> > > Signed-off-by: Ján Tomko 
> > > ---
> > ...
> > 
> > > 
> > >  for (i = 0; i < sharedFolders.count; i++) {
> > >  ISharedFolder *sharedFolder = sharedFolders.items[i];
> > > @@ -3798,10 +3789,8 @@ vboxDumpAudio(virDomainDefPtr def, vboxDriverPtr 
> > > data G_GNUC_UNUSED,
> > >  PRUint32 audioController = AudioControllerType_AC97;
> > > 
> > >  def->nsounds = 1;
> > > -if (VIR_ALLOC_N(def->sounds, def->nsounds) < 0)
> > > -return;
> > > -if (VIR_ALLOC(def->sounds[0]) < 0)
> > > -return;
> > > +def->sounds = g_new0(virDomainSoundDefPtr, def->nsounds);
> > 
> > just use 1 for the counter
> 
> I can do that.
> 
> > and drop the nsounds attribute initialization
> > completely.
> 
> def->nsounds needs to be set to 1, otherwise virDomainDefFormat would
> not even look at def->sounds.

Brainfart...yes, what I actually wanted to write originally was to set
def->nsounds to 1 after the allocation to follow some logical pattern, somehow
ended up writing down a different thought, doesn't matter, just use 1 in the
g_new0 allocator and move on :).

Erik



Re: [libvirt PATCH v2] docs: use "::" instead of ".. code-block::"

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 11:47:26AM +0100, Daniel P. Berrangé wrote:
> The former is a short hand for the latter and is already widely used in
> the docs. Using the short hand avoids incompatibility with the alternate
> impl of rst2html5.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---

Reviewed-by: Erik Skultety 



[libvirt PATCH] rpm: drop ia64, sparc64 and alpha architectures

2020-10-05 Thread Daniel P . Berrangé
None of these arches are relevant to current Fedora or RHEL distros.

Signed-off-by: Daniel P. Berrangé 
---
 libvirt.spec.in | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index aa2bc84be9..682d43c290 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -97,8 +97,8 @@
 
 # Finally set the OS / architecture specific special cases
 
-# Xen is available only on i386 x86_64 ia64
-%ifnarch %{ix86} x86_64 ia64
+# Xen is available only on i386 x86_64
+%ifnarch %{ix86} x86_64
 %define with_libxl 0
 %endif
 
@@ -438,7 +438,7 @@ Requires: iproute-tc
 %endif
 
 Requires: polkit >= 0.112
-%ifarch %{ix86} x86_64 ia64
+%ifarch %{ix86} x86_64
 # For virConnectGetSysinfo
 Requires: dmidecode
 %endif
@@ -1261,7 +1261,7 @@ rm -f 
$RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug
 # Copied into libvirt-docs subpackage eventually
 mv $RPM_BUILD_ROOT%{_datadir}/doc/libvirt libvirt-docs
 
-%ifarch %{power64} s390x x86_64 ia64 alpha sparc64
+%ifarch %{power64} s390x x86_64
 mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes.stp \
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes-64.stp
 
-- 
2.26.2



Re: [libvirt PATCH 9/9] tests: Add tests for unknown elements and attributes in cpu defintion

2020-10-05 Thread Peter Krempa
On Mon, Oct 05, 2020 at 13:41:52 +0200, Ján Tomko wrote:
> On a Monday in 2020, Tim Wiederhake wrote:
> > On Wed, 2020-09-30 at 14:11 +0200, Peter Krempa wrote:
> > > On Wed, Sep 30, 2020 at 13:54:58 +0200, Tim Wiederhake wrote:
> > > > On Tue, 2020-09-29 at 11:22 +0200, Peter Krempa wrote:
> > > > > On Mon, Sep 21, 2020 at 15:07:32 +0200, Tim Wiederhake wrote:
> > > > > > Signed-off-by: Tim Wiederhake 
> > > > > > ---
> > > > >
> > > > > [...]
> > > > >
> > > > > > diff --git a/tests/cputestdata/x86_64-bogus-element.xml
> > > > > > b/tests/cputestdata/x86_64-bogus-element.xml
> > > > > > new file mode 100644
> > > > > > index 00..79f98bad18
> > > > > > --- /dev/null
> > > > > > +++ b/tests/cputestdata/x86_64-bogus-element.xml
> > > > > > @@ -0,0 +1,3 @@
> > > > > > +
> > > > > > +  
> > > > > > +
> > > > >
> > > > > I'm not persuaded that there's value in such test.
> > > > >
> > > >
> > > > This was my approach to adding negative tests to the "virsh
> > > > [hypervisor-]cpu-compare --validate" feature to make sure that
> > > > schema
> > > > violations are actually caught. If this is not the correct place to
> > > > do
> > > > so, I would be grateful for a pointer in the right direction.
> > > 
> > > I'd find way more useful to actually test that all the XMLs we use in
> > > the tests are validated, which does not seem to happen in this
> > > series.
> > > 
> > > This series adds schema validation only for a very synthetic negative
> > > case which IMO doesn't make sense.
> > > 
> > > Additionally we do have 'virschematest', which is meant to validate
> > > all
> > > XML documents in the tests. Based on the fact that you've added a
> > > invalid XML example but it's name doesn't end in '-invalid.xml',
> > > which
> > > is the marker for the 'virschematest' worker to do negative
> > > validation,
> > > means that the cpu files are not validated.
> > > 
> > > Rather than adding a synthetic case, wire up virschematest which will
> > > make sure that we don't have bogus XMLs in the test suite.
> > 
> > Thanks, I was unaware of this test. One problem I see is that
> > tests/cputestdata contains xml files with different root elements:
> > 
> > $ for i in tests/cputestdata/*.xml ; do xpath -q -e "name(/*)" $i ;
> > done | sort | uniq -c
> >273 cpu
> >157 cpudata
> > 16 cpuTest
> > 
> > Adding 'DO_TEST_DIR("cpu.rng", "cputestdata");' to virschematest.c
> > therefore fails for 157 + 16 files. I do not believe that we would want
> > to individually list 273 files in virschematest.c and reworking the
> > directory structure appears undesirable.
> 
> If by undesirable you mean you don't want to touch it with a 3.048 m
> pole, I understand.
> 
> But it's not trivial to figure out from cputest.c or the filenames
> which files belong to which tests, so separating them to different
> directories might help readability.
> 
> The 'cpudata' files should have -cpuid- in their name, so separating
> them to a separate directory shouldn't be so difficult.
> 
> And  is used as a wrapper for multiple  nodes for
> baseline tests.

Alternatively we can add a special schema file for tests which will
declare  as a 'oneOrMore' of  elements, and will also
describe .

Or both.

Specifically the special schema will be helpful to validate the
concatenated files for baselining as they really aren't really validated
otherwise if we want to do it via virschematest.

Since the new version of your patches is validating all files and not
just the negative cases it deals with my complaint just fine. I just
didn't see a point in validating negative cases only without actually
trying if we even pass on some files.

Whether it's worth pursuing the change to virschematest is up to you.



Re: [PATCH v2 2/7] virbitmaptest: Split up test4

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

'test4' was testing three distinct operations on separate instances of a
bitmap. Split it up into 'test4a', 'test4b' and 'test4c' so that the
'bitmap' variable is not reused.

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 64 +++
1 file changed, 46 insertions(+), 18 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v2 1/7] virbitmaptest: Split up test12

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

'test12' was testing two distinct operations on two instances of a
bitmap. Split it up into 'test12a' and 'test12b' so that the 'bitmap'


'map'

or just drop the quotes.


variable is not reused.

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v2 3/7] virbitmaptest: Use separate output strings in 'test5'

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

The test validates two outputs. Don't reuse 'str' for both.

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v2 4/7] virbitmaptest: Turn 'TEST_MAP' macro into a helper function

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

The function will also be reusable in other places of the code by making
the size check optional. For now only test12* is refactored since it
used TEST_MAP directly.

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 63 +--
1 file changed, 37 insertions(+), 26 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v2 5/7] virbitmaptest: Refactor checks in 'test6'

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

The 'checkBitmap' helper uses 'virBitmapFormat' internally and also
reports better errors. Use it instead of the open-coded checks.

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 44 ++-
1 file changed, 6 insertions(+), 38 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v2 6/7] virbitmaptest: Use g_auto(free) for cleanup

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 79 ++-
1 file changed, 25 insertions(+), 54 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v2 7/7] virbitmaptest: Remove unnecessary error/cleanup labels

2020-10-05 Thread Ján Tomko

On a Monday in 2020, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---
tests/virbitmaptest.c | 270 +-
1 file changed, 107 insertions(+), 163 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [libvirt PATCH] rpm: drop ia64, sparc64 and alpha architectures

2020-10-05 Thread Andrea Bolognani
On Mon, 2020-10-05 at 13:17 +0100, Daniel P. Berrangé wrote:
> +++ b/libvirt.spec.in
> -%ifarch %{power64} s390x x86_64 ia64 alpha sparc64
> +%ifarch %{power64} s390x x86_64
>  mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes.stp \
> $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes-64.stp

I'm pretty sure aarch64 has systemtap support, so we might want to
reconsider this architecture list.

Anyway that's work for another patch, so

  Reviewed-by: Andrea Bolognani 

for this one.

-- 
Andrea Bolognani / Red Hat / Virtualization



[libvirt PATCH] rpm: include aarch64 & riscv64 in systemtap 64-bit arch tapset rename

2020-10-05 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 libvirt.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 682d43c290..d13aae5cf5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1261,7 +1261,7 @@ rm -f 
$RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug
 # Copied into libvirt-docs subpackage eventually
 mv $RPM_BUILD_ROOT%{_datadir}/doc/libvirt libvirt-docs
 
-%ifarch %{power64} s390x x86_64
+%ifarch %{power64} s390x x86_64 aarch64 riscv64
 mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes.stp \
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes-64.stp
 
-- 
2.26.2



Re: [PATCH] rpm: Enable Xen support on AArch64

2020-10-05 Thread Daniel P . Berrangé
On Mon, Oct 05, 2020 at 12:33:23PM +0200, Ján Tomko wrote:
> On a Sunday in 2020, Neal Gompa wrote:
> > Starting with Linux 5.9, Xen Dom0 works on commonly available
> > AArch64 devices, such as the Raspberry Pi 4.
> > 
> 
> My Fedora 32 does not even have 5.9 yet, does it make sense to allow it
> regardless of the version?

New kernels get pushed to all Fedora stable releases, so I'd expect it
to appear in F32 in time.

The "xen" RPM in Fedora as aarch64 enabled in F32 already, so libxl
is available for libvirt to build against.

> 
> Jano
> 
> > Reference: 
> > https://xenproject.org/2020/09/29/xen-on-raspberry-pi-4-adventures/
> > 
> > Signed-off-by: Neal Gompa 
> > ---
> > libvirt.spec.in | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index aa2bc84be9..470782b23e 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -97,8 +97,8 @@
> > 
> > # Finally set the OS / architecture specific special cases
> > 
> > -# Xen is available only on i386 x86_64 ia64
> > -%ifnarch %{ix86} x86_64 ia64
> > +# Xen is available only on i386 x86_64 ia64 aarch64
> > +%ifnarch %{ix86} x86_64 ia64 aarch64
> > %define with_libxl 0
> > %endif
> > 
> > -- 
> > 2.26.2
> > 



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



Re: [python-libvirt] RFC:PEP 484 type annotation - how to continue

2020-10-05 Thread Daniel P . Berrangé
On Mon, Oct 05, 2020 at 09:12:26AM +0200, Philipp Hahn wrote:
> Hello,
> 
> currently my Merge Quests
>  is stuck
> because there are some open technical discussions:
> 
> 1. The Python binding consists of two layers:
>- libvirtmod* is the low-level C-like API, which mostly is just a
> thin wrapper around the C-library. It is generated from the API.xml files
>   - The high-level libvirt* modules, providing an object oriented API,
> which is also generated from the API.xml files.
>   In both cases there are many exceptions defines via
> libvirt*-override-api.xml or coded in generator.py:
>   - skip_impl: Generate Python code, but skip C implementation
>   - custom_function: Generate neither C nor Python code - hand crafted code
>   - skip_function: Generate neither C nor Python code - not used at all
>   - function_skip_python_impl: Generate C code, but skip python impl
> 
> Previously the functions listed in "skip_impl" were also duplicated in
> the "libvirt-*override-api.xml" files, which is somehow hard to
> maintain. With d19699e48ce9c17481f94b4dc0715e18e05d5adb I removed all
> those names from "skip_impl", which are also listed in the overrides and
> added code to add them back when "generator.py" is invoked.
> For this to work I had to change several "files" attributes to start
> with "python", which looks okay to me, but still would like to get verified.

The generator.py is such a big hack I don't think it matters too much.
The real important thing is that the resulting code is sane.

> 
> 2. The low-level library works mostly with bytes, but the high-level
> Python API uses nested Lists, Tuples and Dicts. To get most out of the
> Python static type annotations the input and output parameters should be
> modled in as much detail as possible.
> This requires to store those types somewhere. My current
> 46592e28124858ed863bceba65c3dafa2bbb02cd adds some often used types to
> generator.py and then references them from the overrides.xml:
> 
> > +'PyListSecret': ('', 'List["virSecret"]', '', ''),
> > +'PyDictAny': ('', 'Dict[str, Any]', '', ''),
> 
> > +  
> 
> This still is very painful as there are many "used-once" types, which
> still must be declared and used in two places.
> So far the "return" type was used for the "C" type and currently I
> mis-use it for referencing it for my Python types. This doesn't look
> right, so I would prefer adding a new attribute which can be used to
> override the Python type.
> This also would probably nicely fit with functions like
> "virDomainOpenConsole", when "dev_name" is optional and can be "None",
> which the current type hint requires a "string".
> 
> Is extending the libvirt*-override-api.xml files okay or must this be
> coordinated with the other parts of the project?

These files follow the same schema as the main api.xml files shipped by
libvirt itself. That is just a matter of convenience though - the
overrides are only used in the libvirt-python generator.py, so there
is no reason why we can't record extra information in them.


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



Re: [PATCH] docs/submitting-patches: add reference to DCO

2020-10-05 Thread Mauro Matteo Cascella
On Fri, Oct 2, 2020 at 3:49 PM Peter Krempa  wrote:
>
> On Fri, Oct 02, 2020 at 15:32:48 +0200, Mauro Matteo Cascella wrote:
> > Signed-off-by: Mauro Matteo Cascella 
> > ---
> >  docs/submitting-patches.rst | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/submitting-patches.rst b/docs/submitting-patches.rst
> > index 17b072655d..3ce8b347a0 100644
> > --- a/docs/submitting-patches.rst
> > +++ b/docs/submitting-patches.rst
> > @@ -22,7 +22,8 @@ patch. However, the usual workflow of libvirt developer 
> > is:
> >(hack, committing any changes along the way)
> >
> >  More hints on compiling can be found `here `__.
> > -When you want to post your patches:
> > +Make sure to `sign off `__
> > +your commit(s). When you want to post your patches:
>
> IMO this doesn't express the underlying certification of complience
> enough.
>
> I'd suggest something like:
>
> Make sure to express your agreement with the 'Developer Certificate of
> Origin'  by adding your 'sign-off'.
>

I didn't think too much about the form, just wanted to put a reference
to the DCO paragraph where this is already well explained. That being
said, I agree the sentence could be better rephrased. Will send a new
version of the patch, thanks for your suggestion.

--
Mauro Matteo Cascella, Red Hat Product Security
6F78 E20B 5935 928C F0A8  1A9D 4E55 23B8 BB34 10B0



[PATCH v2] docs/submitting-patches: add reference to DCO

2020-10-05 Thread Mauro Matteo Cascella
Signed-off-by: Mauro Matteo Cascella 
---
Rephrased sentence.

 docs/submitting-patches.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/submitting-patches.rst b/docs/submitting-patches.rst
index 17b072655d..7cb5c2e172 100644
--- a/docs/submitting-patches.rst
+++ b/docs/submitting-patches.rst
@@ -22,6 +22,9 @@ patch. However, the usual workflow of libvirt developer is:
   (hack, committing any changes along the way)
 
 More hints on compiling can be found `here `__.
+Make sure to express your agreement with the `Developer Certificate
+of Origin `__ by
+adding a "Signed-off-by" line to every commit message.
 When you want to post your patches:
 
 ::
-- 
2.26.2



Re: [libvirt PATCH 1/2] esx: call freeaddrinfo earlier in esxUtil_ResolveHostname

2020-10-05 Thread Laine Stump

On 10/5/20 7:41 AM, Pino Toscano wrote:

Call freeaddrinfo() as soon as @result is not needed anymore, i.e. right
after getnameinfo(); this avoids calling freeaddrinfo() in two branches.

Signed-off-by: Pino Toscano 



Reviewed-by: Laine Stump 


---
  src/esx/esx_util.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 9100873326..555158f953 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -310,17 +310,15 @@ esxUtil_ResolveHostname(const char *hostname,
  
  errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,

ipAddress_length, NULL, 0, NI_NUMERICHOST);
+freeaddrinfo(result);
  
  if (errcode != 0) {

  virReportError(VIR_ERR_INTERNAL_ERROR,
 _("Formatting IP address for host '%s' failed: %s"), 
hostname,
 gai_strerror(errcode));
-freeaddrinfo(result);
  return -1;
  }
  
-freeaddrinfo(result);

-
  return 0;
  }
  





Re: [libvirt PATCH 2/2] esx: switch esxUtil_ResolveHostname to return a new string

2020-10-05 Thread Laine Stump

On 10/5/20 7:41 AM, Pino Toscano wrote:

Change the interface of esxUtil_ResolveHostname() to return a newly
allocated string with the result address, instead of forcing the callers
to provide a buffer and its size. This way we can simply (auto)free the
string, and make the function stacks smaller.

Signed-off-by: Pino Toscano 



Reviewed-by: Laine Stump 



---
  src/esx/esx_driver.c | 20 +++-
  src/esx/esx_util.c   | 11 +++
  src/esx/esx_util.h   |  3 +--
  3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index e82e5ed835..a17bf58a51 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -602,7 +602,7 @@ esxConnectToHost(esxPrivate *priv,
   char **vCenterIPAddress)
  {
  int result = -1;
-char ipAddress[NI_MAXHOST] = "";
+g_autofree char *ipAddress = NULL;
  char *username = NULL;
  char *password = NULL;
  char *url = NULL;
@@ -615,7 +615,7 @@ esxConnectToHost(esxPrivate *priv,
  
  ESX_VI_CHECK_ARG_LIST(vCenterIPAddress);
  
-if (esxUtil_ResolveHostname(conn->uri->server, ipAddress, NI_MAXHOST) < 0)

+if (esxUtil_ResolveHostname(conn->uri->server, &ipAddress) < 0)
  return -1;
  
  if (conn->uri->user) {

@@ -692,7 +692,7 @@ esxConnectToVCenter(esxPrivate *priv,
  const char *hostSystemIPAddress)
  {
  int result = -1;
-char ipAddress[NI_MAXHOST] = "";
+g_autofree char *ipAddress = NULL;
  char *username = NULL;
  char *password = NULL;
  char *url = NULL;
@@ -704,7 +704,7 @@ esxConnectToVCenter(esxPrivate *priv,
  return -1;
  }
  
-if (esxUtil_ResolveHostname(hostname, ipAddress, NI_MAXHOST) < 0)

+if (esxUtil_ResolveHostname(hostname, &ipAddress) < 0)
  return -1;
  
  if (conn->uri->user) {

@@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
  virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
  esxPrivate *priv = NULL;
  char *potentialVCenterIPAddress = NULL;
-char vCenterIPAddress[NI_MAXHOST] = "";
+g_autofree char *vCenterIPAddress = NULL;
  
  virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
  
@@ -875,16 +875,10 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,

  goto cleanup;
  }
  
-if (virStrcpyStatic(vCenterIPAddress,

-potentialVCenterIPAddress) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("vCenter IP address %s too big for 
destination"),
-   potentialVCenterIPAddress);
-goto cleanup;
-}
+vCenterIPAddress = g_strdup(potentialVCenterIPAddress);
  } else {
  if (esxUtil_ResolveHostname(priv->parsedUri->vCenter,
-vCenterIPAddress, NI_MAXHOST) < 0) 
{
+&vCenterIPAddress) < 0) {
  goto cleanup;
  }
  
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c

index 555158f953..d9e7641d67 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -278,12 +278,12 @@ esxUtil_ParseDatastorePath(const char *datastorePath, 
char **datastoreName,
  
  
  int

-esxUtil_ResolveHostname(const char *hostname,
-char *ipAddress, size_t ipAddress_length)
+esxUtil_ResolveHostname(const char *hostname, char **ipAddress)
  {
  struct addrinfo hints;
  struct addrinfo *result = NULL;
  int errcode;
+g_autofree char *address = NULL;
  
  memset(&hints, 0, sizeof(hints));
  
@@ -308,8 +308,9 @@ esxUtil_ResolveHostname(const char *hostname,

  return -1;
  }
  
-errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,

-  ipAddress_length, NULL, 0, NI_NUMERICHOST);
+address = g_new0(char, NI_MAXHOST);
+errcode = getnameinfo(result->ai_addr, result->ai_addrlen, address,
+  NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
  freeaddrinfo(result);
  
  if (errcode != 0) {

@@ -319,6 +320,8 @@ esxUtil_ResolveHostname(const char *hostname,
  return -1;
  }
  
+*ipAddress = g_strdup(address);

+
  return 0;
  }
  
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h

index 97b6d82a2b..9bfbff1d42 100644
--- a/src/esx/esx_util.h
+++ b/src/esx/esx_util.h
@@ -55,8 +55,7 @@ int esxUtil_ParseVirtualMachineIDString(const char 
*id_string, int *id);
  int esxUtil_ParseDatastorePath(const char *datastorePath, char 
**datastoreName,
 char **directoryName, char 
**directoryAndFileName);
  
-int esxUtil_ResolveHostname(const char *hostname,

-char *ipAddress, size_t ipAddress_length);
+int esxUtil_ResolveHostname(const char *hostname, char **ipAddress);
  
  int e

Re: [libvirt PATCH 0/9] qemu: use g_new0 (glib chronicles)

2020-10-05 Thread Erik Skultety
On Mon, Oct 05, 2020 at 12:36:30PM +0200, Ján Tomko wrote:
> With the exception of qemu_migrate_cookie and qemu_slirp which
> should be dealt with by Peter's series.

[1-5, 6v2, 7-9]:
Reviewed-by: Erik Skultety 



Re: [PATCH v2 0/4] Add support for QEMU's 'fmode' and 'dmode'

2020-10-05 Thread Brian Turek
I did some further testing and it appears that QEMU is reading the flags as
base-10 which causes issues. I'll need to make some tweaks and release a v3
of the patches later today.

Please ignore this set.

On Fri, Oct 2, 2020, 11:55 AM Brian Turek  wrote:

> This second version of the patch incorporates Peter Krempa's feedback
> by tweaking a few things in the code and, more importantly, splitting
> up the singular large patch into smaller patches.
>
> Brian Turek (4):
>   qemu: capabilities: add QEMU_CAPS_FSDEV_CREATEMODE
>   qemu: add support for 'fmode' and 'dmode' options
>   qemu: add schema 'fmode' and 'dmode' options
>   qemu: add docs for 'fmode' and 'dmode' options
>
>  docs/formatdomain.rst | 12 
>  docs/schemas/domaincommon.rng | 16 +
>  src/conf/domain_conf.c| 27 
>  src/conf/domain_conf.h|  2 +
>  src/qemu/qemu_capabilities.c  |  2 +
>  src/qemu/qemu_capabilities.h  |  1 +
>  src/qemu/qemu_command.c   |  6 ++
>  src/qemu/qemu_validate.c  | 18 ++
>  .../caps_2.10.0.aarch64.xml   |  1 +
>  .../caps_2.10.0.ppc64.xml |  1 +
>  .../caps_2.10.0.s390x.xml |  1 +
>  .../caps_2.10.0.x86_64.xml|  1 +
>  .../caps_2.11.0.s390x.xml |  1 +
>  .../caps_2.11.0.x86_64.xml|  1 +
>  .../caps_2.12.0.aarch64.xml   |  1 +
>  .../caps_2.12.0.ppc64.xml |  1 +
>  .../caps_2.12.0.s390x.xml |  1 +
>  .../caps_2.12.0.x86_64.xml|  1 +
>  .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 +
>  .../caps_3.0.0.riscv32.xml|  1 +
>  .../caps_3.0.0.riscv64.xml|  1 +
>  .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 +
>  .../caps_3.0.0.x86_64.xml |  1 +
>  .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 +
>  .../caps_3.1.0.x86_64.xml |  1 +
>  .../caps_4.0.0.aarch64.xml|  1 +
>  .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |  1 +
>  .../caps_4.0.0.riscv32.xml|  1 +
>  .../caps_4.0.0.riscv64.xml|  1 +
>  .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  1 +
>  .../caps_4.0.0.x86_64.xml |  1 +
>  .../caps_4.1.0.x86_64.xml |  1 +
>  .../caps_4.2.0.aarch64.xml|  1 +
>  .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |  1 +
>  .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  1 +
>  .../caps_4.2.0.x86_64.xml |  1 +
>  .../caps_5.0.0.aarch64.xml|  1 +
>  .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  1 +
>  .../caps_5.0.0.riscv64.xml|  1 +
>  .../caps_5.0.0.x86_64.xml |  1 +
>  .../caps_5.1.0.x86_64.xml |  1 +
>  .../caps_5.2.0.x86_64.xml |  1 +
>  .../virtio-9p-createmode.x86_64-latest.args   | 45 ++
>  .../qemuxml2argvdata/virtio-9p-createmode.xml | 58 ++
>  .../virtio-9p-createmode.x86_64-latest.xml| 61 +++
>  tests/qemuxml2xmltest.c   |  1 +
>  46 files changed, 283 insertions(+)
>  create mode 100644
> tests/qemuxml2argvdata/virtio-9p-createmode.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.xml
>  create mode 100644
> tests/qemuxml2xmloutdata/virtio-9p-createmode.x86_64-latest.xml
>
> --
> 2.25.1
>
>


Re: [PATCH 1/1] Optional free-page-reporting can be enabled/disabled for memballon device of model 'virtio'.

2020-10-05 Thread Nico Pache
Thanks for the reviews!

I'll get started on a V2 :)

--Nico

On Mon, Oct 5, 2020 at 5:32 AM Peter Krempa  wrote:

> On Mon, Oct 05, 2020 at 11:11:54 +0200, David Hildenbrand wrote:
> > On 05.10.20 10:06, Peter Krempa wrote:
> > > On Fri, Oct 02, 2020 at 11:59:43 -0400, Nico Pache wrote:
> > >> xml:
> > >> 
> > >>   
> > >
> > > according to what you state in the cover-letter this is a
> > > 'virtio-balloon-pci' feature. We usually put stuff which depends on a
> > > specific model of the device into the  subelement of the device
> > > element.
> >
> > IIRC, this should work for virtio-balloon-*, including
> virtio-balloon-ccw.
>
> In that case the qemu capability detection should be added also for the
> ccw version and capability name needs to be modified to not include pci.
>
>


Re: [libvirt PATCH] rpm: include aarch64 & riscv64 in systemtap 64-bit arch tapset rename

2020-10-05 Thread Andrea Bolognani
On Mon, 2020-10-05 at 13:54 +0100, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [PATCH] tests: cover disk, interface

2020-10-05 Thread Michal Privoznik

On 10/4/20 9:05 PM, Cole Robinson wrote:

There is present no XML test coverage for this.
Add genericxml parse + formatting coverage.

Signed-off-by: Cole Robinson 
---
  .../device-backenddomain.xml  | 30 +++
  .../device-backenddomain.xml  |  1 +
  tests/genericxml2xmltest.c|  1 +
  3 files changed, 32 insertions(+)
  create mode 100644 tests/genericxml2xmlindata/device-backenddomain.xml
  create mode 12 tests/genericxml2xmloutdata/device-backenddomain.xml


Reviewed-by: Michal Privoznik 

Michal



Re: [PATCH V4] Modify virCPUarmCompare to perform compare actions

2020-10-05 Thread Michal Privoznik

On 10/2/20 3:45 PM, Daniel Henrique Barboza wrote:



On 9/24/20 11:12 AM, Zhenyu Zheng wrote:

Modify virCPUarmCompare in cpu_arm.c to perform compare action.
This patch only adds host to host CPU compare, the rest cases
remains the same. This is useful for source and destination host
compare during migrations to avoid migration between different
CPU models that have different CPU freatures.

Signed-off-by: Zhenyu Zheng 
---


Reviewed-by: Daniel Henrique Barboza 


Pushed now.

Michal



Re: [libvirt PATCH] rpm: include aarch64 & riscv64 in systemtap 64-bit arch tapset rename

2020-10-05 Thread Neal Gompa
On Mon, Oct 5, 2020 at 8:55 AM Daniel P. Berrangé  wrote:
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 682d43c290..d13aae5cf5 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1261,7 +1261,7 @@ rm -f 
> $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug
>  # Copied into libvirt-docs subpackage eventually
>  mv $RPM_BUILD_ROOT%{_datadir}/doc/libvirt libvirt-docs
>
> -%ifarch %{power64} s390x x86_64
> +%ifarch %{power64} s390x x86_64 aarch64 riscv64
>  mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes.stp \
> $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_probes-64.stp
>
> --
> 2.26.2
>

Reviewed-by: Neal Gompa 


-- 
真実はいつも一つ!/ Always, there's only one truth!




Re: [PATCH] rpm: Enable Xen support on AArch64

2020-10-05 Thread Neal Gompa
On Mon, Oct 5, 2020 at 6:18 AM Andrea Bolognani  wrote:
>
> On Sun, 2020-10-04 at 22:16 -0400, Neal Gompa wrote:
> > +++ b/libvirt.spec.in
> > +# Xen is available only on i386 x86_64 ia64 aarch64
> > +%ifnarch %{ix86} x86_64 ia64 aarch64
> >  %define with_libxl 0
> >  %endif
>
> The code change is okay, but duplicating the list of architectures in
> the comment is kinda pointless and results in unnecessary churn; I
> would reword it as
>
>   Xen is available only on some architectures
>
> Are you okay with that change? If so, I will add my
>
>   Reviewed-by: Andrea Bolognani 
>
> and push the patch.
>

I'm fine with that. I debated making that change myself, too. :)

-- 
真実はいつも一つ!/ Always, there's only one truth!




[libvirt PATCH 1/4] virsh: do not add bools into size calculations

2020-10-05 Thread Ján Tomko
Switch the allocation in virshSnapshotListCollect and
its cargo-culted Checkpoint counterpart to two separate
g_new0 calls and move the boolean expression to
the if condition that chooses between them.

Signed-off-by: Ján Tomko 
---
 tools/virsh-checkpoint.c | 6 --
 tools/virsh-snapshot.c   | 6 --
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
index f3c4fe90ba..cefdfd7800 100644
--- a/tools/virsh-checkpoint.c
+++ b/tools/virsh-checkpoint.c
@@ -600,8 +600,10 @@ virshCheckpointListCollect(vshControl *ctl,
 
 /* When mixing --from and --tree, we also want a copy of from
  * in the list, but with no parent for that one entry.  */
-checkpointlist->chks = vshCalloc(ctl, count + (tree && from),
- sizeof(*checkpointlist->chks));
+if (from && tree)
+checkpointlist->chks = g_new0(struct virshChk, count + 1);
+else
+checkpointlist->chks = g_new0(struct virshChk, count);
 checkpointlist->nchks = count;
 for (i = 0; i < count; i++)
 checkpointlist->chks[i].chk = chks[i];
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 99e3d98c6f..b4498df298 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1102,8 +1102,10 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr 
dom,
 if (count >= 0) {
 /* When mixing --from and --tree, we also want a copy of from
  * in the list, but with no parent for that one entry.  */
-snaplist->snaps = vshCalloc(ctl, count + (tree && from),
-sizeof(*snaplist->snaps));
+if (tree && from)
+snaplist->snaps = g_new0(struct virshSnap, count + 1);
+else
+snaplist->snaps = g_new0(struct virshSnap, count);
 snaplist->nsnaps = count;
 for (i = 0; i < count; i++)
 snaplist->snaps[i].snap = snaps[i];
-- 
2.26.2



[libvirt PATCH 4/4] virsh: network-port: remove pointless comment

2020-10-05 Thread Ján Tomko
We do not have a legacy API for listing network ports
so there's nothing to fall back on.

Signed-off-by: Ján Tomko 
---
 tools/virsh-network.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index c5347660de..745afc537d 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1693,7 +1693,6 @@ virshNetworkPortListCollect(vshControl *ctl,
 if (!(network = virshCommandOptNetwork(ctl, cmd, NULL)))
 goto cleanup;
 
-/* try the list with flags support (0.10.2 and later) */
 if ((ret = virNetworkListAllPorts(network,
   &list->ports,
   flags)) < 0)
-- 
2.26.2



[libvirt PATCH 2/4] virsh: use g_new0 instead of vsh[CM]alloc

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 tools/virsh-checkpoint.c |  5 +++--
 tools/virsh-domain-monitor.c | 11 +--
 tools/virsh-domain.c | 24 
 tools/virsh-host.c   | 22 +++---
 tools/virsh-interface.c  |  8 
 tools/virsh-network.c|  8 
 tools/virsh-nodedev.c| 12 ++--
 tools/virsh-nwfilter.c   |  8 
 tools/virsh-pool.c   |  8 
 tools/virsh-secret.c |  6 +++---
 tools/virsh-snapshot.c   |  8 
 tools/virsh-volume.c |  8 
 tools/vsh.c  | 12 ++--
 13 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
index cefdfd7800..ac9d5bd348 100644
--- a/tools/virsh-checkpoint.c
+++ b/tools/virsh-checkpoint.c
@@ -583,11 +583,12 @@ virshCheckpointListCollect(vshControl *ctl,
 size_t i;
 int count = -1;
 virDomainCheckpointPtr *chks;
-virshCheckpointListPtr checkpointlist = vshMalloc(ctl,
-  sizeof(*checkpointlist));
+virshCheckpointListPtr checkpointlist = NULL;
 virshCheckpointListPtr ret = NULL;
 unsigned int flags = orig_flags;
 
+checkpointlist = g_new0(struct virshCheckpointList, 1);
+
 if (from)
 count = virDomainCheckpointListAllChildren(from, &chks, flags);
 else
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index c8a7c0f1b7..e0491d48ac 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1063,8 +1063,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
 DOMBLKSTAT_LEGACY_PRINT(3, stats.wr_bytes);
 DOMBLKSTAT_LEGACY_PRINT(4, stats.errs);
 } else {
-params = vshCalloc(ctl, nparams, sizeof(*params));
-
+params = g_new0(virTypedParameter, nparams);
 if (virDomainBlockStatsFlags(dom, device, params, &nparams, 0) < 0) {
 vshError(ctl, _("Failed to get block stats for domain '%s' device 
'%s'"), name, device);
 goto cleanup;
@@ -1618,7 +1617,7 @@ virshDomainListFree(virshDomainListPtr domlist)
 static virshDomainListPtr
 virshDomainListCollect(vshControl *ctl, unsigned int flags)
 {
-virshDomainListPtr list = vshMalloc(ctl, sizeof(*list));
+virshDomainListPtr list = g_new0(struct virshDomainList, 1);
 size_t i;
 int ret;
 int *ids = NULL;
@@ -1680,7 +1679,7 @@ virshDomainListCollect(vshControl *ctl, unsigned int 
flags)
 }
 
 if (nids) {
-ids = vshMalloc(ctl, sizeof(int) * nids);
+ids = g_new0(int, nids);
 
 if ((nids = virConnectListDomains(priv->conn, ids, nids)) < 0) {
 vshError(ctl, "%s", _("Failed to list active domains"));
@@ -1697,7 +1696,7 @@ virshDomainListCollect(vshControl *ctl, unsigned int 
flags)
 }
 
 if (nnames) {
-names = vshMalloc(ctl, sizeof(char *) * nnames);
+names = g_new0(char *, nnames);
 
 if ((nnames = virConnectListDefinedDomains(priv->conn, names,
   nnames)) < 0) {
@@ -1707,7 +1706,7 @@ virshDomainListCollect(vshControl *ctl, unsigned int 
flags)
 }
 }
 
-list->domains = vshMalloc(ctl, sizeof(virDomainPtr) * (nids + nnames));
+list->domains = g_new0(virDomainPtr, nids + nnames);
 list->ndomains = 0;
 
 /* get active domains */
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b5e7d8f705..8f11393197 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1431,7 +1431,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }
 
-params = vshCalloc(ctl, nparams, sizeof(*params));
+params = g_new0(virTypedParameter, nparams);
 
 if (virDomainGetBlockIoTune(dom, disk, params, &nparams, flags) != 0) {
 vshError(ctl, "%s",
@@ -1629,7 +1629,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
 }
 
 /* now go get all the blkio parameters */
-params = vshCalloc(ctl, nparams, sizeof(*params));
+params = g_new0(virTypedParameter, nparams);
 if (virDomainGetBlkioParameters(dom, params, &nparams, flags) != 0) {
 vshError(ctl, "%s", _("Unable to get blkio parameters"));
 goto cleanup;
@@ -2372,7 +2372,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd)
 transientjob) {
 /* New API */
 if (bandwidth || granularity || buf_size) {
-params = vshCalloc(ctl, 3, sizeof(*params));
+params = g_new0(virTypedParameter, 3);
 if (bandwidth) {
 if (!bytes) {
 /* bandwidth is ulong MiB/s, but the typed parameter is
@@ -3375,7 +3375,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
 }
 
 /* get all interface parameters */
-params = vshCalloc(ctl, nparams, 

[libvirt PATCH 0/4] virsh: use g_new0 (glib chronicles)

2020-10-05 Thread Ján Tomko
Ján Tomko (4):
  virsh: do not add bools into size calculations
  virsh: use g_new0 instead of vsh[CM]alloc
  virsh: delete vsh[CM]alloc
  virsh: network-port: remove pointless comment

 tools/virsh-checkpoint.c | 11 +++
 tools/virsh-domain-monitor.c | 11 +--
 tools/virsh-domain.c | 24 
 tools/virsh-host.c   | 22 +++---
 tools/virsh-interface.c  |  8 
 tools/virsh-network.c|  9 -
 tools/virsh-nodedev.c| 12 ++--
 tools/virsh-nwfilter.c   |  8 
 tools/virsh-pool.c   |  8 
 tools/virsh-secret.c |  6 +++---
 tools/virsh-snapshot.c   | 14 --
 tools/virsh-volume.c |  8 
 tools/vsh.c  | 30 ++
 tools/vsh.h  |  6 --
 14 files changed, 78 insertions(+), 99 deletions(-)

-- 
2.26.2



[libvirt PATCH 3/4] virsh: delete vsh[CM]alloc

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 tools/vsh.c | 18 --
 tools/vsh.h |  6 --
 2 files changed, 24 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 98066d17bf..ca92bcd78c 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -100,24 +100,6 @@ vshPrettyCapacity(unsigned long long val, const char 
**unit)
 }
 
 
-void *
-_vshMalloc(vshControl *ctl, size_t size, const char *filename, int line)
-{
-char *x;
-
-if (VIR_ALLOC_N(x, size) == 0)
-return x;
-vshError(ctl, _("%s: %d: failed to allocate %d bytes"),
- filename, line, (int) size);
-exit(EXIT_FAILURE);
-}
-
-void *
-vshCalloc(vshControl *ctl G_GNUC_UNUSED, size_t nmemb, size_t size)
-{
-return g_malloc0_n(nmemb, size);
-}
-
 int
 vshNameSorter(const void *a, const void *b)
 {
diff --git a/tools/vsh.h b/tools/vsh.h
index 0a40995bf5..5583fef9cc 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -465,12 +465,6 @@ char * vshReadline(vshControl *ctl, const char *prompt);
 
 void vshReadlineHistoryAdd(const char *cmd);
 
-/* allocation wrappers */
-void *_vshMalloc(vshControl *ctl, size_t sz, const char *filename, int line);
-#define vshMalloc(_ctl, _sz)_vshMalloc(_ctl, _sz, __FILE__, __LINE__)
-
-void *vshCalloc(vshControl *ctl, size_t nmemb, size_t sz);
-
 /* Macros to help dealing with mutually exclusive options. */
 
 /* VSH_EXCLUSIVE_OPTIONS_EXPR:
-- 
2.26.2



[libvirt PATCH 02/12] util: split out VIR_ALLOC calls

2020-10-05 Thread Ján Tomko
To make the following commits simpler.

Signed-off-by: Ján Tomko 
---
 src/util/vircommand.c |  6 --
 src/util/virxml.c | 10 --
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 859d6b0ce5..8060cdfada 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2636,8 +2636,10 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
 VIR_FORCE_CLOSE(cmd->infd);
 /* clear any error so we can catch if the helper thread reports one */
 cmd->has_error = 0;
-if (VIR_ALLOC(cmd->asyncioThread) < 0 ||
-virThreadCreateFull(cmd->asyncioThread, true,
+if (VIR_ALLOC(cmd->asyncioThread) < 0)
+ret = -1;
+
+if (virThreadCreateFull(cmd->asyncioThread, true,
 virCommandDoAsyncIOHelper,
 "cmd-async-io", false, cmd) < 0) {
 virReportSystemError(errno, "%s",
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 5315d4ff6f..1927ff490f 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -672,12 +672,10 @@ virXPathNodeSet(const char *xpath,
 
 ret = obj->nodesetval->nodeNr;
 if (list != NULL && ret) {
-if (VIR_ALLOC_N(*list, ret) < 0) {
-ret = -1;
-} else {
-memcpy(*list, obj->nodesetval->nodeTab,
-   ret * sizeof(xmlNodePtr));
-}
+if (VIR_ALLOC_N(*list, ret) < 0)
+return -1;
+
+memcpy(*list, obj->nodesetval->nodeTab, ret * sizeof(xmlNodePtr));
 }
 xmlXPathFreeObject(obj);
 return ret;
-- 
2.26.2



[libvirt PATCH 01/12] util: resctrl fix spacing in comment

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virresctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index aea3fe8687..2535627d63 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -1879,7 +1879,7 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
 }
 }
 
-/* set default free memory bandwidth to 100%*/
+/* set default free memory bandwidth to 100% */
 if (info->membw_info) {
 if (VIR_ALLOC(ret->mem_bw) < 0)
 goto error;
-- 
2.26.2



[libvirt PATCH 05/12] util: sysinfo: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virsysinfo.c | 33 +++--
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 0e9b281caf..217f842a37 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -225,8 +225,7 @@ virSysinfoParsePPCSystem(const char *base, 
virSysinfoSystemDefPtr *sysdef)
 if ((cur = strstr(base, "platform")) == NULL)
 return 0;
 
-if (VIR_ALLOC(def) < 0)
-return ret;
+def = g_new0(virSysinfoSystemDef, 1);
 
 base = cur;
 /* Account for format 'platform: '*/
@@ -318,8 +317,7 @@ virSysinfoReadPPC(void)
 g_auto(virSysinfoDefPtr) ret = NULL;
 g_autofree char *outbuf = NULL;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virSysinfoDef, 1);
 
 if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -350,8 +348,7 @@ virSysinfoParseARMSystem(const char *base, 
virSysinfoSystemDefPtr *sysdef)
 if ((cur = strstr(base, "platform")) == NULL)
 return 0;
 
-if (VIR_ALLOC(def) < 0)
-return ret;
+def = g_new0(virSysinfoSystemDef, 1);
 
 base = cur;
 /* Account for format 'platform: '*/
@@ -453,8 +450,7 @@ virSysinfoReadARM(void)
 /* Well, we've tried. Fall back to parsing cpuinfo */
 virResetLastError();
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virSysinfoDef, 1);
 
 if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -507,8 +503,7 @@ virSysinfoParseS390System(const char *base, 
virSysinfoSystemDefPtr *sysdef)
 int ret = -1;
 virSysinfoSystemDefPtr def;
 
-if (VIR_ALLOC(def) < 0)
-return ret;
+def = g_new0(virSysinfoSystemDef, 1);
 
 if (!virSysinfoParseS390Line(base, "Manufacturer", &def->manufacturer))
 goto cleanup;
@@ -612,8 +607,7 @@ virSysinfoReadS390(void)
 g_auto(virSysinfoDefPtr) ret = NULL;
 g_autofree char *outbuf = NULL;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virSysinfoDef, 1);
 
 /* Gather info from /proc/cpuinfo */
 if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) {
@@ -653,8 +647,7 @@ virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr 
*bios)
 if ((cur = strstr(base, "BIOS Information")) == NULL)
 return 0;
 
-if (VIR_ALLOC(def) < 0)
-return ret;
+def = g_new0(virSysinfoBIOSDef, 1);
 
 base = cur;
 if ((cur = strstr(base, "Vendor: ")) != NULL) {
@@ -710,8 +703,7 @@ virSysinfoParseX86System(const char *base, 
virSysinfoSystemDefPtr *sysdef)
 if ((cur = strstr(base, "System Information")) == NULL)
 return 0;
 
-if (VIR_ALLOC(def) < 0)
-return ret;
+def = g_new0(virSysinfoSystemDef, 1);
 
 base = cur;
 if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
@@ -877,8 +869,7 @@ virSysinfoParseX86Chassis(const char *base,
 if ((cur = strstr(base, "Chassis Information")) == NULL)
 return 0;
 
-if (VIR_ALLOC(def) < 0)
-return ret;
+def = g_new0(virSysinfoChassisDef, 1);
 
 base = cur;
 if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
@@ -968,8 +959,7 @@ virSysinfoParseOEMStrings(const char *base,
 if (!(cur = strstr(base, "OEM Strings")))
 return 0;
 
-if (VIR_ALLOC(strings) < 0)
-return -1;
+strings = g_new0(virSysinfoOEMStringsDef, 1);
 
 while ((cur = strstr(cur, "String "))) {
 char *eol;
@@ -1237,8 +1227,7 @@ virSysinfoReadDMI(void)
 if (virCommandRun(cmd, NULL) < 0)
 return NULL;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virSysinfoDef, 1);
 
 ret->type = VIR_SYSINFO_SMBIOS;
 
-- 
2.26.2



[libvirt PATCH 03/12] util: resctrl: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virresctrl.c | 77 ++-
 1 file changed, 25 insertions(+), 52 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 2535627d63..400c8e9981 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -552,9 +552,7 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
 continue;
 }
 
-if (VIR_ALLOC(i_type) < 0)
-goto cleanup;
-
+i_type = g_new0(virResctrlInfoPerType, 1);
 i_type->control.scope = type;
 
 rv = virFileReadValueUint(&i_type->control.max_allocation,
@@ -617,13 +615,9 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
 if (!resctrl->levels[level]) {
 virResctrlInfoPerTypePtr *types = NULL;
 
-if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0)
-goto cleanup;
+types = g_new0(virResctrlInfoPerTypePtr, VIR_CACHE_TYPE_LAST);
 
-if (VIR_ALLOC(resctrl->levels[level]) < 0) {
-VIR_FREE(types);
-goto cleanup;
-}
+resctrl->levels[level] = g_new0(virResctrlInfoPerLevel, 1);
 resctrl->levels[level]->types = types;
 }
 
@@ -654,8 +648,7 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
 virResctrlInfoMemBWPtr i_membw = NULL;
 
 /* query memory bandwidth allocation info */
-if (VIR_ALLOC(i_membw) < 0)
-goto cleanup;
+i_membw = g_new0(virResctrlInfoMemBW, 1);
 rv = virFileReadValueUint(&i_membw->bandwidth_granularity,
   SYSFS_RESCTRL_PATH "/info/MB/bandwidth_gran");
 if (rv == -2) {
@@ -721,8 +714,7 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 size_t nfeatures = 0;
 virResctrlInfoMongrpPtr info_monitor = NULL;
 
-if (VIR_ALLOC(info_monitor) < 0)
-return -1;
+info_monitor = g_new0(virResctrlInfoMongrp, 1);
 
 /* For now, monitor only exists in level 3 cache */
 info_monitor->cache_level = 3;
@@ -947,9 +939,7 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
 
 if (VIR_EXPAND_N(*controls, *ncontrols, 1) < 0)
 goto error;
-if (VIR_ALLOC((*controls)[*ncontrols - 1]) < 0)
-goto error;
-
+(*controls)[*ncontrols - 1] = g_new0(virResctrlInfoPerCache, 1);
 memcpy((*controls)[*ncontrols - 1], &i_type->control, 
sizeof(i_type->control));
 }
 
@@ -1004,8 +994,7 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfoPtr resctrl,
 
 for (i = 0; i < VIR_RESCTRL_MONITOR_TYPE_LAST; i++) {
 if (STREQ(prefix, virResctrlMonitorPrefixTypeToString(i))) {
-if (VIR_ALLOC(mon) < 0)
-goto cleanup;
+mon = g_new0(virResctrlInfoMon, 1);
 mon->type = i;
 break;
 }
@@ -1121,20 +1110,16 @@ virResctrlAllocGetType(virResctrlAllocPtr alloc,
 if (!alloc->levels[level]) {
 virResctrlAllocPerTypePtr *types = NULL;
 
-if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0)
-return NULL;
+types = g_new0(virResctrlAllocPerTypePtr, VIR_CACHE_TYPE_LAST);
 
-if (VIR_ALLOC(alloc->levels[level]) < 0) {
-VIR_FREE(types);
-return NULL;
-}
+alloc->levels[level] = g_new0(virResctrlAllocPerLevel, 1);
 alloc->levels[level]->types = types;
 }
 
 a_level = alloc->levels[level];
 
-if (!a_level->types[type] && VIR_ALLOC(a_level->types[type]) < 0)
-return NULL;
+if (!a_level->types[type])
+a_level->types[type] = g_new0(virResctrlAllocPerType, 1);
 
 return a_level->types[type];
 }
@@ -1181,8 +1166,8 @@ virResctrlAllocUpdateSize(virResctrlAllocPtr alloc,
  cache - a_type->nsizes + 1) < 0)
 return -1;
 
-if (!a_type->sizes[cache] && VIR_ALLOC(a_type->sizes[cache]) < 0)
-return -1;
+if (!a_type->sizes[cache])
+a_type->sizes[cache] = g_new0(unsigned long long, 1);
 
 *(a_type->sizes[cache]) = size;
 
@@ -1332,8 +1317,7 @@ virResctrlAllocSetMemoryBandwidth(virResctrlAllocPtr 
alloc,
 }
 
 if (!mem_bw) {
-if (VIR_ALLOC(mem_bw) < 0)
-return -1;
+mem_bw = g_new0(virResctrlAllocMemBW, 1);
 alloc->mem_bw = mem_bw;
 }
 
@@ -1349,9 +1333,7 @@ virResctrlAllocSetMemoryBandwidth(virResctrlAllocPtr 
alloc,
 return -1;
 }
 
-if (VIR_ALLOC(mem_bw->bandwidths[id]) < 0)
-return -1;
-
+mem_bw->bandwidths[id] = g_new0(unsigned int, 1);
 *(mem_bw->bandwidths[id]) = memory_bandwidth;
 return 0;
 }
@@ -1497,10 +1479,8 @@ 
virResctrlAllocParseProcessMemoryBandwidth(virResctrlInfoPtr resctrl,
  id - alloc->mem_bw->nbandwidths + 1) < 0) {
 return -1;
 }
-if (!alloc->mem_bw->bandwidths[id]) {
-if (VIR_ALLOC(alloc->mem_bw->bandwidths[id]) < 0)
-return -1;
-}
+if (!alloc->mem_bw->bandwidths[id])
+allo

[libvirt PATCH 07/12] util: netdev: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virnetdev.c | 25 +
 src/util/virnetdevbandwidth.c| 22 --
 src/util/virnetdevip.c   |  3 +--
 src/util/virnetdevmacvlan.c  |  6 ++
 src/util/virnetdevtap.c  |  3 +--
 src/util/virnetdevvlan.c |  4 +---
 src/util/virnetdevvportprofile.c |  7 ++-
 7 files changed, 20 insertions(+), 50 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 76aeeba22a..e711a6dc8a 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1209,8 +1209,7 @@ virNetDevGetVirtualFunctions(const char *pfname,
   n_vfname, max_vfs) < 0)
 goto cleanup;
 
-if (VIR_ALLOC_N(*vfname, *n_vfname) < 0)
-goto cleanup;
+*vfname = g_new0(char *, *n_vfname);
 
 for (i = 0; i < *n_vfname; i++) {
 g_autofree char *pciConfigAddr = NULL;
@@ -2039,8 +2038,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
 }
 
 if (MACStr) {
-if (VIR_ALLOC(*MAC) < 0)
-goto cleanup;
+*MAC = g_new0(virMacAddr, 1);
 
 if (virMacAddrParse(MACStr, *MAC) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2051,8 +2049,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
 }
 
 if (adminMACStr) {
-if (VIR_ALLOC(*adminMAC) < 0)
-goto cleanup;
+*adminMAC = g_new0(virMacAddr, 1);
 
 if (virMacAddrParse(adminMACStr, *adminMAC) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2064,10 +2061,8 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
 
 if (vlanTag != -1) {
 /* construct a simple virNetDevVlan object with a single tag */
-if (VIR_ALLOC(*vlan) < 0)
-goto cleanup;
-if (VIR_ALLOC((*vlan)->tag) < 0)
-goto cleanup;
+*vlan = g_new0(virNetDevVlan, 1);
+(*vlan)->tag = g_new0(unsigned int, 1);
 (*vlan)->nTags = 1;
 (*vlan)->tag[0] = vlanTag;
 }
@@ -2664,8 +2659,8 @@ static int virNetDevGetMcastList(const char *ifname,
 
 cur = buf;
 while (cur) {
-if (!entry && VIR_ALLOC(entry) < 0)
-return -1;
+if (!entry)
+entry = g_new0(virNetDevMcastEntry, 1);
 
 next = strchr(cur, '\n');
 if (next)
@@ -2709,8 +2704,7 @@ static int virNetDevGetMulticastTable(const char *ifname,
 goto cleanup;
 
 if (mcast.nentries > 0) {
-if (VIR_ALLOC_N(filter->multicast.table, mcast.nentries) < 0)
-goto cleanup;
+filter->multicast.table = g_new0(virMacAddr, mcast.nentries);
 
 for (i = 0; i < mcast.nentries; i++) {
 virMacAddrSet(&filter->multicast.table[i],
@@ -2733,8 +2727,7 @@ virNetDevRxFilterNew(void)
 {
 virNetDevRxFilterPtr filter;
 
-if (VIR_ALLOC(filter) < 0)
-return NULL;
+filter = g_new0(virNetDevRxFilter, 1);
 return filter;
 }
 
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 5fd7186760..c8eb5cfc79 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -442,39 +442,25 @@ int
 virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest,
const virNetDevBandwidth *src)
 {
-int ret = -1;
-
 *dest = NULL;
 if (!src) {
 /* nothing to be copied */
 return 0;
 }
 
-if (VIR_ALLOC(*dest) < 0)
-goto cleanup;
+*dest = g_new0(virNetDevBandwidth, 1);
 
 if (src->in) {
-if (VIR_ALLOC((*dest)->in) < 0)
-goto cleanup;
+(*dest)->in = g_new0(virNetDevBandwidthRate, 1);
 memcpy((*dest)->in, src->in, sizeof(*src->in));
 }
 
 if (src->out) {
-if (VIR_ALLOC((*dest)->out) < 0) {
-VIR_FREE((*dest)->in);
-goto cleanup;
-}
+(*dest)->out = g_new0(virNetDevBandwidthRate, 1);
 memcpy((*dest)->out, src->out, sizeof(*src->out));
 }
 
-ret = 0;
-
- cleanup:
-if (ret < 0) {
-virNetDevBandwidthFree(*dest);
-*dest = NULL;
-}
-return ret;
+return 0;
 }
 
 bool
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index a69567da6f..fc7808cbaa 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -180,8 +180,7 @@ virNetDevIPAddrAdd(const char *ifname,
 if (VIR_SOCKET_ADDR_FAMILY(addr) == AF_INET &&
 !(peer && VIR_SOCKET_ADDR_VALID(peer))) {
 /* compute a broadcast address if this is IPv4 */
-if (VIR_ALLOC(broadcast) < 0)
-return -1;
+broadcast = g_new0(virSocketAddr, 1);
 
 if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index f301a9e96c..72f0d67086 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -726,11 +726,9 @@ virNetDevMacVLanVPortProfil

[libvirt PATCH 08/12] util: systemd: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virsystemd.c | 19 +--
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 8456085476..8373ee6509 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -364,8 +364,7 @@ int virSystemdCreateMachine(const char *name,
 if (g_atomic_int_get(&hasCreateWithNetwork)) {
 g_autoptr(virError) error = NULL;
 
-if (VIR_ALLOC(error) < 0)
-return -1;
+error = g_new0(virError, 1);
 
 guuid = g_variant_new_fixed_array(G_VARIANT_TYPE("y"),
   uuid, 16, sizeof(unsigned char));
@@ -495,8 +494,7 @@ int virSystemdTerminateMachine(const char *name)
 if (!(conn = virGDBusGetSystemBus()))
 return -1;
 
-if (VIR_ALLOC(error) < 0)
-return -1;
+error = g_new0(virError, 1);
 
 /*
  * The systemd DBus API we're invoking has the
@@ -655,14 +653,8 @@ virSystemdActivationAddFD(virSystemdActivationPtr act,
 virSystemdActivationEntryPtr ent = virHashLookup(act->fds, name);
 
 if (!ent) {
-if (VIR_ALLOC(ent) < 0)
-return -1;
-
-if (VIR_ALLOC_N(ent->fds, 1) < 0) {
-virSystemdActivationEntryFree(ent);
-return -1;
-}
-
+ent = g_new0(virSystemdActivationEntry, 1);
+ent->fds = g_new0(int, 1);
 ent->fds[ent->nfds++] = fd;
 
 VIR_DEBUG("Record first FD %d with name %s", fd, name);
@@ -902,8 +894,7 @@ virSystemdActivationNew(virSystemdActivationMap *map,
 const char *fdnames;
 
 VIR_DEBUG("Activated with %d FDs", nfds);
-if (VIR_ALLOC(act) < 0)
-return NULL;
+act = g_new0(virSystemdActivation, 1);
 
 if (!(act->fds = virHashCreate(10, virSystemdActivationEntryFree)))
 goto error;
-- 
2.26.2



[libvirt PATCH 04/12] util: storagefile: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virstoragefile.c | 82 ---
 1 file changed, 25 insertions(+), 57 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 459a7be5e4..82388ae544 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -520,8 +520,7 @@ qcow2GetExtensions(const char *buf,
 if (!backingFormat)
 break;
 
-if (VIR_ALLOC_N(tmp, len + 1) < 0)
-return -1;
+tmp = g_new0(char, len + 1);
 memcpy(tmp, buf + offset, len);
 tmp[len] = '\0';
 
@@ -575,8 +574,7 @@ qcowXGetBackingStore(char **res,
 return BACKING_STORE_INVALID;
 if (offset + size > buf_size || offset + size < offset)
 return BACKING_STORE_INVALID;
-if (VIR_ALLOC_N(*res, size + 1) < 0)
-return BACKING_STORE_ERROR;
+*res = g_new0(char, size + 1);
 memcpy(*res, buf + offset, size);
 (*res)[size] = '\0';
 
@@ -598,8 +596,7 @@ vmdk4GetBackingStore(char **res,
 size_t len;
 g_autofree char *desc = NULL;
 
-if (VIR_ALLOC_N(desc, VIR_STORAGE_MAX_HEADER) < 0)
-return BACKING_STORE_ERROR;
+desc = g_new0(char, VIR_STORAGE_MAX_HEADER);
 
 *res = NULL;
 /*
@@ -669,8 +666,7 @@ qedGetBackingStore(char **res,
 return BACKING_STORE_OK;
 if (offset + size > buf_size || offset + size < offset)
 return BACKING_STORE_INVALID;
-if (VIR_ALLOC_N(*res, size + 1) < 0)
-return BACKING_STORE_ERROR;
+*res = g_new0(char, size + 1);
 memcpy(*res, buf + offset, size);
 (*res)[size] = '\0';
 
@@ -959,9 +955,7 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
   buf, len)) {
 int expt_fmt = fileTypeInfo[meta->format].cryptInfo[i].format;
 if (!meta->encryption) {
-if (VIR_ALLOC(meta->encryption) < 0)
-return -1;
-
+meta->encryption = g_new0(virStorageEncryption, 1);
 meta->encryption->format = expt_fmt;
 } else {
 if (meta->encryption->format != expt_fmt) {
@@ -1735,8 +1729,7 @@ virStorageNetHostDefCopy(size_t nhosts,
 virStorageNetHostDefPtr ret = NULL;
 size_t i;
 
-if (VIR_ALLOC_N(ret, nhosts) < 0)
-goto error;
+ret = g_new0(virStorageNetHostDef, nhosts);
 
 for (i = 0; i < nhosts; i++) {
 virStorageNetHostDefPtr src = &hosts[i];
@@ -1750,10 +1743,6 @@ virStorageNetHostDefCopy(size_t nhosts,
 }
 
 return ret;
-
- error:
-virStorageNetHostDefFree(nhosts, ret);
-return NULL;
 }
 
 
@@ -1775,8 +1764,7 @@ virStorageAuthDefCopy(const virStorageAuthDef *src)
 {
 g_autoptr(virStorageAuthDef) authdef = NULL;
 
-if (VIR_ALLOC(authdef) < 0)
-return NULL;
+authdef = g_new0(virStorageAuthDef, 1);
 
 authdef->username = g_strdup(src->username);
 /* Not present for storage pool, but used for disk source */
@@ -1801,8 +1789,7 @@ virStorageAuthDefParse(xmlNodePtr node,
 
 ctxt->node = node;
 
-if (VIR_ALLOC(authdef) < 0)
-goto cleanup;
+authdef = g_new0(virStorageAuthDef, 1);
 
 if (!(authdef->username = virXPathString("string(./@username)", ctxt))) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1891,8 +1878,7 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt)
 g_autofree char *path = NULL;
 g_autofree char *mode = NULL;
 
-if (VIR_ALLOC(prd) < 0)
-return NULL;
+prd = g_new0(virStoragePRDef, 1);
 
 if (!(managed = virXPathString("string(./@managed)", ctxt))) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -2020,8 +2006,7 @@ virStoragePRDefCopy(virStoragePRDefPtr src)
 virStoragePRDefPtr copy = NULL;
 virStoragePRDefPtr ret = NULL;
 
-if (VIR_ALLOC(copy) < 0)
-return NULL;
+copy = g_new0(virStoragePRDef, 1);
 
 copy->managed = src->managed;
 
@@ -2129,8 +2114,7 @@ virStorageSourceSeclabelsCopy(virStorageSourcePtr to,
 if (from->nseclabels == 0)
 return 0;
 
-if (VIR_ALLOC_N(to->seclabels, from->nseclabels) < 0)
-return -1;
+to->seclabels = g_new0(virSecurityDeviceLabelDefPtr, from->nseclabels);
 to->nseclabels = from->nseclabels;
 
 for (i = 0; i < to->nseclabels; i++) {
@@ -2280,8 +2264,7 @@ virStorageTimestampsCopy(const virStorageTimestamps *src)
 {
 virStorageTimestampsPtr ret;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virStorageTimestamps, 1);
 
 memcpy(ret, src, sizeof(*src));
 
@@ -2294,8 +2277,7 @@ virStoragePermsCopy(const virStoragePerms *src)
 {
 virStoragePermsPtr ret;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virStoragePerms, 1);
 
 ret->mode = src->mode;
 ret->uid = src->uid;
@@ -2312,8 +2294,7 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef 
*src)
 {
 virStorageSource

[libvirt PATCH 00/12] util: use g_new0 (glib chronicles)

2020-10-05 Thread Ján Tomko
Ján Tomko (12):
  util: resctrl fix spacing in comment
  util: split out VIR_ALLOC calls
  util: resctrl: use g_new0
  util: storagefile: use g_new0
  util: sysinfo: use g_new0
  util: command: use g_new0
  util: netdev: use g_new0
  util: systemd: use g_new0
  util: firewall: use g_new0
  util: conf: use g_new0
  util: a-n: use g_new0
  util: o-z: use g_new0

 src/util/iohelper.c  |  3 +-
 src/util/virarptable.c   |  3 +-
 src/util/virauthconfig.c |  6 +--
 src/util/virbitmap.c |  4 +-
 src/util/vircgroup.c |  7 +--
 src/util/vircommand.c| 31 +---
 src/util/virconf.c   | 23 +++--
 src/util/vircrypto.c |  6 +--
 src/util/virdnsmasq.c| 12 ++---
 src/util/virebtables.c   |  3 +-
 src/util/virfile.c   | 12 ++---
 src/util/virfirewall.c   |  9 ++--
 src/util/virfirewalld.c  |  6 +--
 src/util/virfirmware.c   |  6 +--
 src/util/virhash.c   |  8 +---
 src/util/virhostcpu.c|  9 ++--
 src/util/virjson.c   | 15 ++
 src/util/virlockspace.c  | 17 ++-
 src/util/virlog.c|  5 +-
 src/util/virmdev.c   |  6 +--
 src/util/virnetdev.c | 25 --
 src/util/virnetdevbandwidth.c| 22 ++---
 src/util/virnetdevip.c   |  3 +-
 src/util/virnetdevmacvlan.c  |  6 +--
 src/util/virnetdevtap.c  |  3 +-
 src/util/virnetdevvlan.c |  4 +-
 src/util/virnetdevvportprofile.c |  7 +--
 src/util/virnetlink.c|  3 +-
 src/util/virnuma.c   |  7 +--
 src/util/virpci.c| 12 ++---
 src/util/virperf.c   |  3 +-
 src/util/virpolkit.c |  3 +-
 src/util/virportallocator.c  |  3 +-
 src/util/virprocess.c|  3 +-
 src/util/virresctrl.c| 79 ++
 src/util/virrotatingfile.c   | 15 ++
 src/util/virscsi.c   |  6 +--
 src/util/virscsivhost.c  |  3 +-
 src/util/virseclabel.c   | 13 ++---
 src/util/virstorageencryption.c  | 16 ++-
 src/util/virstoragefile.c| 82 ++--
 src/util/virstring.c |  3 +-
 src/util/virsysinfo.c| 33 +
 src/util/virsystemd.c| 19 ++--
 src/util/virthreadpool.c |  9 ++--
 src/util/virtime.c   |  6 +--
 src/util/virtypedparam.c | 20 +++-
 src/util/viruri.c|  3 +-
 src/util/virusb.c|  3 +-
 src/util/virutil.c   | 15 ++
 src/util/virxml.c| 11 ++---
 51 files changed, 196 insertions(+), 435 deletions(-)

-- 
2.26.2



[libvirt PATCH 10/12] util: conf: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virconf.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/src/util/virconf.c b/src/util/virconf.c
index 8575dd8849..e983a769ee 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -163,8 +163,7 @@ virConfNew(void)
 {
 virConfPtr ret;
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virConf, 1);
 ret->filename = NULL;
 ret->flags = 0;
 
@@ -219,8 +218,7 @@ virConfAddEntry(virConfPtr conf, char *name, 
virConfValuePtr value, char *comm)
 if (name)
 VIR_DEBUG("Add entry %s %p", name, value);
 
-if (VIR_ALLOC(ret) < 0)
-return NULL;
+ret = g_new0(virConfEntry, 1);
 
 ret->name = name;
 ret->value = value;
@@ -522,11 +520,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
 virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting a value"));
 return NULL;
 }
-if (VIR_ALLOC(ret) < 0) {
-virConfFreeList(lst);
-VIR_FREE(str);
-return NULL;
-}
+ret = g_new0(virConfValue, 1);
 ret->type = type;
 ret->l = l;
 ret->str = str;
@@ -947,8 +941,7 @@ int virConfGetValueStringList(virConfPtr conf,
 }
 }
 
-if (VIR_ALLOC_N(*values, len + 1) < 0)
-return -1;
+*values = g_new0(char *, len + 1);
 
 for (len = 0, eval = cval->list; eval; len++, eval = eval->next)
 (*values)[len] = g_strdup(eval->str);
@@ -956,8 +949,7 @@ int virConfGetValueStringList(virConfPtr conf,
 
 case VIR_CONF_STRING:
 if (compatString) {
-if (VIR_ALLOC_N(*values, cval->str ? 2 : 1) < 0)
-return -1;
+*values = g_new0(char *, cval->str ? 2 : 1);
 if (cval->str)
 (*values)[0] = g_strdup(cval->str);
 break;
@@ -1356,10 +1348,7 @@ virConfSetValue(virConfPtr conf,
 }
 
 if (!cur) {
-if (VIR_ALLOC(cur) < 0) {
-virConfFreeValue(value);
-return -1;
-}
+cur = g_new0(virConfEntry, 1);
 cur->comment = NULL;
 cur->name = g_strdup(setting);
 cur->value = value;
-- 
2.26.2



[libvirt PATCH 09/12] util: firewall: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virfirewall.c  | 9 +++--
 src/util/virfirewalld.c | 6 ++
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 22e717bce4..f6a8beec95 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -211,8 +211,7 @@ virFirewallGroupNew(void)
 {
 virFirewallGroupPtr group;
 
-if (VIR_ALLOC(group) < 0)
-return NULL;
+group = g_new0(virFirewallGroup, 1);
 
 return group;
 }
@@ -235,8 +234,7 @@ virFirewallPtr virFirewallNew(void)
 if (virFirewallInitialize() < 0)
 return NULL;
 
-if (VIR_ALLOC(firewall) < 0)
-return NULL;
+firewall = g_new0(virFirewall, 1);
 
 return firewall;
 }
@@ -346,8 +344,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 group = firewall->groups[firewall->currentGroup];
 
 
-if (VIR_ALLOC(rule) < 0)
-goto no_memory;
+rule = g_new0(virFirewallRule, 1);
 
 rule->layer = layer;
 rule->queryCB = cb;
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index a94ac7c183..3178bf4b3d 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -139,8 +139,7 @@ virFirewallDGetBackend(void)
 if (!sysbus)
 return -1;
 
-if (VIR_ALLOC(error) < 0)
-return -1;
+error = g_new0(virError, 1);
 
 message = g_variant_new("(ss)",
 "org.fedoraproject.FirewallD1.config",
@@ -289,8 +288,7 @@ virFirewallDApplyRule(virFirewallLayer layer,
 return -1;
 }
 
-if (VIR_ALLOC(error) < 0)
-return -1;
+error = g_new0(virError, 1);
 
 message = g_variant_new("(s@as)",
 ipv,
-- 
2.26.2



[libvirt PATCH 11/12] util: a-n: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/iohelper.c  |  3 +--
 src/util/virarptable.c   |  3 +--
 src/util/virauthconfig.c |  6 ++
 src/util/virbitmap.c |  4 +---
 src/util/vircgroup.c |  7 ++-
 src/util/vircrypto.c |  6 ++
 src/util/virdnsmasq.c| 12 
 src/util/virebtables.c   |  3 +--
 src/util/virfile.c   | 12 +++-
 src/util/virfirmware.c   |  6 ++
 src/util/virhash.c   |  8 ++--
 src/util/virhostcpu.c|  9 +++--
 src/util/virjson.c   | 15 +--
 src/util/virlockspace.c  | 17 +
 src/util/virlog.c|  5 +
 src/util/virmdev.c   |  6 ++
 src/util/virnetlink.c|  3 +--
 src/util/virnuma.c   |  7 ++-
 18 files changed, 40 insertions(+), 92 deletions(-)

diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index c3eb6f8c54..49d939d290 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -63,8 +63,7 @@ runIO(const char *path, int fd, int oflags)
 }
 buf = base;
 #else
-if (VIR_ALLOC_N(buf, buflen + alignMask) < 0)
-goto cleanup;
+buf = g_new0(char, buflen + alignMask);
 base = buf;
 buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
 #endif
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index c4b46604a9..01a27c0093 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -74,8 +74,7 @@ virArpTableGet(void)
 if (msglen < 0)
 return NULL;
 
-if (VIR_ALLOC(table) < 0)
-return NULL;
+table = g_new0(virArpTable, 1);
 
 nh = (struct nlmsghdr*)nlData;
 
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 2e50609531..6b5487c4b0 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -40,8 +40,7 @@ virAuthConfigPtr virAuthConfigNew(const char *path)
 {
 virAuthConfigPtr auth;
 
-if (VIR_ALLOC(auth) < 0)
-goto error;
+auth = g_new0(virAuthConfig, 1);
 
 auth->path = g_strdup(path);
 
@@ -65,8 +64,7 @@ virAuthConfigPtr virAuthConfigNewData(const char *path,
 {
 virAuthConfigPtr auth;
 
-if (VIR_ALLOC(auth) < 0)
-goto error;
+auth = g_new0(virAuthConfig, 1);
 
 auth->path = g_strdup(path);
 
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 319bd7b7ce..4fde6f4fd2 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -704,9 +704,7 @@ virBitmapToData(virBitmapPtr bitmap,
 else
 len = (len + CHAR_BIT) / CHAR_BIT;
 
-if (VIR_ALLOC_N(*data, len) < 0)
-return -1;
-
+*data = g_new0(unsigned char, len);
 *dataLen = len;
 
 virBitmapToDataBuf(bitmap, *data, *dataLen);
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 08758b5306..5c0543c66a 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -664,10 +664,8 @@ virCgroupNew(pid_t pid,
 {
 VIR_DEBUG("pid=%lld path=%s parent=%p controllers=%d group=%p",
   (long long) pid, path, parent, controllers, group);
-*group = NULL;
 
-if (VIR_ALLOC((*group)) < 0)
-goto error;
+*group = g_new0(virCgroup, 1);
 
 if (path[0] == '/' || !parent) {
 (*group)->path = g_strdup(path);
@@ -2170,8 +2168,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
 param_idx = 1;
 
 if (guestvcpus && param_idx < nparams) {
-if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0)
-goto cleanup;
+sum_cpu_time = g_new0(unsigned long long, need_cpus);
 if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time,
   need_cpus, cpumap) < 0)
 goto cleanup;
diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c
index 90aed32c53..c4874550af 100644
--- a/src/util/vircrypto.c
+++ b/src/util/vircrypto.c
@@ -88,8 +88,7 @@ virCryptoHashString(virCryptoHash hash,
 
 hashstrlen = (rc * 2) + 1;
 
-if (VIR_ALLOC_N(*output, hashstrlen) < 0)
-return -1;
+*output = g_new0(char, hashstrlen);
 
 for (i = 0; i < rc; i++) {
 (*output)[i * 2] = hex[(buf[i] >> 4) & 0xf];
@@ -167,8 +166,7 @@ virCryptoEncryptDataAESgnutls(gnutls_cipher_algorithm_t 
gnutls_enc_alg,
  * data from non-padded data. Hence datalen + 1
  */
 ciphertextlen = VIR_ROUND_UP(datalen + 1, 16);
-if (VIR_ALLOC_N(ciphertext, ciphertextlen) < 0)
-return -1;
+ciphertext = g_new0(uint8_t, ciphertextlen);
 memcpy(ciphertext, data, datalen);
 
  /* Fill in the padding of the buffer with the size of the padding
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 44aa7ad95d..9030f9218a 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -109,8 +109,7 @@ addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
 goto error;
 
 idx = addnhostsfile->nhosts;
-if (VIR_ALLOC(addnhostsfile->hosts[idx].hostnames) < 0)
-goto error;
+addnhostsfile->hosts[idx].hostnames = g_new0(char *, 1);
 
 addnhostsfile->hosts[idx].ip =

[libvirt PATCH 12/12] util: o-z: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/virpci.c   | 12 
 src/util/virperf.c  |  3 +--
 src/util/virpolkit.c|  3 +--
 src/util/virportallocator.c |  3 +--
 src/util/virprocess.c   |  3 +--
 src/util/virrotatingfile.c  | 15 +--
 src/util/virscsi.c  |  6 ++
 src/util/virscsivhost.c |  3 +--
 src/util/virseclabel.c  | 13 +++--
 src/util/virstorageencryption.c | 16 +---
 src/util/virstring.c|  3 +--
 src/util/virthreadpool.c|  9 +++--
 src/util/virtime.c  |  6 ++
 src/util/virtypedparam.c| 20 +++-
 src/util/viruri.c   |  3 +--
 src/util/virusb.c   |  3 +--
 src/util/virutil.c  | 15 +--
 src/util/virxml.c   |  7 ++-
 18 files changed, 46 insertions(+), 97 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 47c671daa0..6fa8acd246 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1372,8 +1372,7 @@ virPCIDeviceNew(unsigned int domain,
 g_autofree char *vendor = NULL;
 g_autofree char *product = NULL;
 
-if (VIR_ALLOC(dev) < 0)
-return NULL;
+dev = g_new0(virPCIDevice, 1);
 
 dev->address.domain = domain;
 dev->address.bus = bus;
@@ -1422,8 +1421,7 @@ virPCIDeviceCopy(virPCIDevicePtr dev)
 {
 virPCIDevicePtr copy;
 
-if (VIR_ALLOC(copy) < 0)
-return NULL;
+copy = g_new0(virPCIDevice, 1);
 
 /* shallow copy to take care of most attributes */
 *copy = *dev;
@@ -1871,8 +1869,7 @@ virPCIGetIOMMUGroupAddressesAddOne(virPCIDeviceAddressPtr 
newDevAddr, void *opaq
 virPCIDeviceAddressPtr copyAddr;
 
 /* make a copy to insert onto the list */
-if (VIR_ALLOC(copyAddr) < 0)
-goto cleanup;
+copyAddr = g_new0(virPCIDeviceAddress, 1);
 
 *copyAddr = *newDevAddr;
 
@@ -2204,8 +2201,7 @@ virPCIGetDeviceAddressFromSysfsLink(const char 
*device_link)
 }
 
 config_address = g_path_get_basename(device_path);
-if (VIR_ALLOC(bdf) < 0)
-return NULL;
+bdf = g_new0(virPCIDeviceAddress, 1);
 
 if (virPCIDeviceAddressParse(config_address, bdf) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virperf.c b/src/util/virperf.c
index 2e95509caf..7f260d2da6 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -368,8 +368,7 @@ virPerfNew(void)
 size_t i;
 virPerfPtr perf;
 
-if (VIR_ALLOC(perf) < 0)
-return NULL;
+perf = g_new0(virPerf, 1);
 
 for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
 perf->events[i].fd = -1;
diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index aad924a065..1d6be443f7 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -178,8 +178,7 @@ virPolkitAgentCreate(void)
 if (virPipe(pipe_fd) < 0)
 goto error;
 
-if (VIR_ALLOC(agent) < 0)
-goto error;
+agent = g_new0(virPolkitAgent, 1);
 
 agent->cmd = virCommandNewArgList(PKTTYAGENT, "--process", NULL);
 
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index f450740318..76c6e43726 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -102,8 +102,7 @@ virPortAllocatorRangeNew(const char *name,
 return NULL;
 }
 
-if (VIR_ALLOC(range) < 0)
-return NULL;
+range = g_new0(virPortAllocatorRange, 1);
 
 range->start = start;
 range->end = end;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index d379f7446f..9366f0630c 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1351,8 +1351,7 @@ virProcessNamespaceAvailable(unsigned int ns)
 /* Signal parent as soon as the child dies. RIP. */
 flags |= SIGCHLD;
 
-if (VIR_ALLOC_N(stack, stacksize) < 0)
-return -1;
+stack = g_new0(char, stacksize);
 
 childStack = stack + stacksize;
 
diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c
index b77e30dba7..a88c332cf4 100644
--- a/src/util/virrotatingfile.c
+++ b/src/util/virrotatingfile.c
@@ -105,8 +105,7 @@ virRotatingFileWriterEntryNew(const char *path,
 
 VIR_DEBUG("Opening %s mode=0%02o", path, mode);
 
-if (VIR_ALLOC(entry) < 0)
-return NULL;
+entry = g_new0(virRotatingFileWriterEntry, 1);
 
 if ((entry->fd = open(path, O_CREAT|O_APPEND|O_WRONLY|O_CLOEXEC, mode)) < 
0) {
 virReportSystemError(errno,
@@ -148,8 +147,7 @@ virRotatingFileReaderEntryNew(const char *path)
 
 VIR_DEBUG("Opening %s", path);
 
-if (VIR_ALLOC(entry) < 0)
-return NULL;
+entry = g_new0(virRotatingFileReaderEntry, 1);
 
 if ((entry->fd = open(path, O_RDONLY|O_CLOEXEC)) < 0) {
 if (errno != ENOENT) {
@@ -238,8 +236,7 @@ virRotatingFileWriterNew(const char *path,
 {
 virRotatingFileWriterPtr file;
 
-if (VIR_ALLOC(file) < 0)
-goto error;
+file = g_new0(virRotatingFileWriter, 1);
 
   

[libvirt PATCH 06/12] util: command: use g_new0

2020-10-05 Thread Ján Tomko
Signed-off-by: Ján Tomko 
---
 src/util/vircommand.c | 29 +
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 8060cdfada..6350e77523 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -935,8 +935,7 @@ virCommandNewArgs(const char *const*args)
 {
 virCommandPtr cmd;
 
-if (VIR_ALLOC(cmd) < 0)
-return NULL;
+cmd = g_new0(virCommand, 1);
 
 cmd->handshakeWait[0] = -1;
 cmd->handshakeWait[1] = -1;
@@ -2183,21 +2182,18 @@ virCommandProcessIO(virCommandPtr cmd)
 if (cmd->outbuf) {
 outfd = cmd->outfd;
 VIR_FREE(*cmd->outbuf);
-if (VIR_ALLOC_N(*cmd->outbuf, 1) < 0)
-ret = -1;
+*cmd->outbuf = g_new0(char, 1);
 }
 if (cmd->errbuf) {
 errfd = cmd->errfd;
 VIR_FREE(*cmd->errbuf);
-if (VIR_ALLOC_N(*cmd->errbuf, 1) < 0)
-ret = -1;
+*cmd->errbuf = g_new0(char, 1);
 }
 if (ret == -1)
 goto cleanup;
 ret = -1;
 
-if (VIR_ALLOC_N(fds, 3 + virCommandGetNumSendBuffers(cmd)) < 0)
-goto cleanup;
+fds = g_new0(struct pollfd, 3 + virCommandGetNumSendBuffers(cmd));
 
 for (;;) {
 size_t i;
@@ -2636,8 +2632,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
 VIR_FORCE_CLOSE(cmd->infd);
 /* clear any error so we can catch if the helper thread reports one */
 cmd->has_error = 0;
-if (VIR_ALLOC(cmd->asyncioThread) < 0)
-ret = -1;
+cmd->asyncioThread = g_new0(virThread, 1);
 
 if (virThreadCreateFull(cmd->asyncioThread, true,
 virCommandDoAsyncIOHelper,
@@ -2854,10 +2849,7 @@ int virCommandHandshakeWait(virCommandPtr cmd)
 if (c != '1') {
 g_autofree char *msg = NULL;
 ssize_t len;
-if (VIR_ALLOC_N(msg, 1024) < 0) {
-VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
-return -1;
-}
+msg = g_new0(char, 1024);
 /* Close the handshakeNotify fd before trying to read anything
  * further on the handshakeWait pipe; so that a child waiting
  * on our acknowledgment will die rather than deadlock.  */
@@ -3193,8 +3185,7 @@ virCommandRunRegex(virCommandPtr cmd,
 int ret = -1;
 
 /* Compile all regular expressions */
-if (VIR_ALLOC_N(reg, nregex) < 0)
-return -1;
+reg = g_new0(GRegex *, nregex);
 
 for (i = 0; i < nregex; i++) {
 g_autoptr(GError) err = NULL;
@@ -3212,8 +3203,7 @@ virCommandRunRegex(virCommandPtr cmd,
 }
 
 /* Storage for matched variables */
-if (VIR_ALLOC_N(groups, totgroups) < 0)
-goto cleanup;
+groups = g_new0(char *, totgroups);
 
 virCommandSetOutputBuffer(cmd, &outbuf);
 if (virCommandRun(cmd, exitstatus) < 0)
@@ -3299,8 +3289,7 @@ virCommandRunNul(virCommandPtr cmd,
 if (n_columns == 0)
 return -1;
 
-if (VIR_ALLOC_N(v, n_columns) < 0)
-return -1;
+v = g_new0(char *, n_columns);
 for (i = 0; i < n_columns; i++)
 v[i] = NULL;
 
-- 
2.26.2



[PATCHv3 4/5] qemu: add docs for 'fmode' and 'dmode' options

2020-10-05 Thread Brian Turek
Adds documentation for QEMU 9pfs 'fmode' and 'dmode' options.

Signed-off-by: Brian Turek 
---
 docs/formatdomain.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index cc4f91d4ea..085f29ef8f 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3062,6 +3062,12 @@ A directory on the host that can be accessed directly 
from the guest.


  
+ 
+   
+   
+   
+   
+ 
  


@@ -3140,6 +3146,12 @@ A directory on the host that can be accessed directly 
from the guest.
"virtio-non-transitional", or "virtio". See `Virtio transitional
devices <#elementsVirtioTransitional>`__ for more details.
 
+   The filesystem element has optional attributes ``fmode`` and ``dmode``.
+   These two attributes control the creation mode for files and directories
+   when used with the ``mapped`` value for ``accessmode`` (:since:`since 6.9.0,
+   requires QEMU 2.10` ).  If not specified, QEMU creates files with mode
+   ``600`` and directories with mode ``700``.
+
The filesystem element has an optional attribute ``multidevs`` which
specifies how to deal with a filesystem export containing more than one
device, in order to avoid file ID collisions on guest when using 9pfs (
-- 
2.25.1



[PATCHv3 1/5] qemu: capabilities: add QEMU_CAPS_FSDEV_CREATEMODE

2020-10-05 Thread Brian Turek
The QEMU 9pfs 'fmode' and 'dmode' options have existed since QEMU 2.10.
Probe QEMU's command line set to check whether these options are
available, and if yes, enable this new QEMU_CAPS_FSDEV_CREATEMODE
capability on libvirt side.

Signed-off-by: Brian Turek 
---
 src/qemu/qemu_capabilities.c   | 2 ++
 src/qemu/qemu_capabilities.h   | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml   | 1 +
 36 files changed, 37 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 38b901a6c4..d2a745d8c5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -600,6 +600,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
   /* 380 */
   "usb-host.hostdevice",
+  "fsdev.createmode",
 );
 
 
@@ -3322,6 +3323,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "smp-opts", "dies", QEMU_CAPS_SMP_DIES },
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
 { "fw_cfg", "file", QEMU_CAPS_FW_CFG },
+{ "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also 
checked fsdev->dmode */
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 107056ba17..bd7412d6f7 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -580,6 +580,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 
 /* 380 */
 QEMU_CAPS_USB_HOST_HOSTDEVICE, /* -device usb-host.hostdevice */
+QEMU_CAPS_FSDEV_CREATEMODE, /* fsdev.createmode */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
index b0fcbc4218..77af6b0d7a 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
@@ -145,6 +145,7 @@
   
   
   
+  
   201
   0
   61700287
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
index edf01d2e2f..5fce7540f9 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
@@ -145,6 +145,7 @@
   
   
   
+  
   201
   0
   42900287
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index 98a3c0eec2..7b0153e272 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -111,6 +111,7 @@
   
   
   
+  
   201
   0
   39100287
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
index 98b1a94349..7f45a473cb 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
@@ -189,6 +189,7 @@
   
   
   
+  
   201
   0
   43100287
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0

[PATCHv3 2/5] qemu: add support for 'fmode' and 'dmode' options

2020-10-05 Thread Brian Turek
Expose QEMU's 9pfs 'fmode' and 'dmode' options via attributes on the
'filesystem' node in the domain XML. These options control the creation
mode of files and directories, respectively, when using
accessmode=mapped.  QEMU defaults to creating files with mode 0600 and
directories with mode 0700.

Signed-off-by: Brian Turek 
---
 src/conf/domain_conf.c| 27 
 src/conf/domain_conf.h|  2 +
 src/qemu/qemu_command.c   |  6 ++
 src/qemu/qemu_validate.c  | 18 ++
 .../virtio-9p-createmode.x86_64-latest.args   | 45 ++
 .../qemuxml2argvdata/virtio-9p-createmode.xml | 58 ++
 .../virtio-9p-createmode.x86_64-latest.xml| 61 +++
 tests/qemuxml2xmltest.c   |  1 +
 8 files changed, 218 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/virtio-9p-createmode.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.xml
 create mode 100644 
tests/qemuxml2xmloutdata/virtio-9p-createmode.x86_64-latest.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 175b632a38..e80b3b7ef6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11496,6 +11496,8 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 g_autofree char *units = NULL;
 g_autofree char *model = NULL;
 g_autofree char *multidevs = NULL;
+g_autofree char *fmode = NULL;
+g_autofree char *dmode = NULL;
 
 ctxt->node = node;
 
@@ -11524,6 +11526,24 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
 }
 
+fmode = virXMLPropString(node, "fmode");
+if (fmode) {
+if (virStrToLong_uip(fmode, NULL, 8, &def->fmode) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid fmode: '%s'"), fmode);
+goto error;
+}
+}
+
+dmode = virXMLPropString(node, "dmode");
+if (dmode) {
+if (virStrToLong_uip(dmode, NULL, 8, &def->dmode) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid dmode: '%s'"), dmode);
+goto error;
+}
+}
+
 model = virXMLPropString(node, "model");
 if (model) {
 if ((def->model = virDomainFSModelTypeFromString(model)) < 0 ||
@@ -26211,6 +26231,13 @@ virDomainFSDefFormat(virBufferPtr buf,
 }
 if (def->multidevs)
 virBufferAsprintf(buf, " multidevs='%s'", multidevs);
+
+if (def->fmode)
+virBufferAsprintf(buf, " fmode='%04o'", def->fmode);
+
+if (def->dmode)
+virBufferAsprintf(buf, " dmode='%04o'", def->dmode);
+
 virBufferAddLit(buf, ">\n");
 
 virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 450686dfb5..51f70f9dd4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -849,6 +849,8 @@ struct _virDomainFSDef {
 int wrpolicy; /* enum virDomainFSWrpolicy */
 int format; /* virStorageFileFormat */
 int model; /* virDomainFSModel */
+unsigned int fmode;
+unsigned int dmode;
 int multidevs; /* virDomainFSMultidevs */
 unsigned long long usage; /* in bytes */
 virStorageSourcePtr src;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 476cf6972e..b2da53c664 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2275,6 +2275,12 @@ qemuBuildFSStr(virDomainFSDefPtr fs)
 } else if (fs->multidevs == VIR_DOMAIN_FS_MULTIDEVS_WARN) {
 virBufferAddLit(&opt, ",multidevs=warn");
 }
+if (fs->fmode) {
+virBufferAsprintf(&opt, ",fmode=%04o", fs->fmode);
+}
+if (fs->dmode) {
+virBufferAsprintf(&opt, ",dmode=%04o", fs->dmode);
+}
 } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE) {
 /* removed since qemu 4.0.0 see v3.1.0-29-g93aee84f57 */
 virBufferAddLit(&opt, "handle");
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a212605579..4757c55e13 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3530,6 +3530,19 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
 return -1;
 }
 
+if ((fs->fmode != 0) || (fs->dmode != 0)) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_CREATEMODE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+_("fmode and dmode are not supported with this QEMU 
binary"));
+return -1;
+}
+if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_MAPPED) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("fmode and dmode must be used with 
accessmode=mapped"));
+return -1;
+}
+}
+
 switch ((virDomainFSDriverType) fs->fsdriver) {
 case VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT:
 case VIR

Re: restrictions for virtiofs (related to commit: 88957116c9 for libvirt 6.9.0)

2020-10-05 Thread Masayoshi Mizuma
On Mon, Oct 05, 2020 at 10:50:08AM +0200, Michal Privoznik wrote:
> On 10/3/20 1:15 AM, Masayoshi Mizuma wrote:
> > On Fri, Oct 02, 2020 at 11:31:32AM -0400, Masayoshi Mizuma wrote:
> > > Hello Jan, and Michal,
> > > 
> > > commit: 88957116c9 ("qemu: Use memory-backend-* for regular guest 
> > > memory") gets
> > > the system memory sharable without numa config.
> > > The qemu options with the patch will be like as:
> > > 
> > >-machine 
> > > pc-q35-5.2,accel=kvm,usb=off,vmport=off,smm=on,dump-guest-core=off,memory-backend=pc.ram
> > >  \
> > >-object 
> > > memory-backend-file,id=pc.ram,mem-path=/var/lib/libvirt/qemu/ram/2-Test/pc.ram,share=yes,size=17179869184
> > >  \
> > > 
> > > So, we can remove the numa restriction of virtiofs, right?
> > > The patch to remove that is the bottom of this email.
> > > 
> > > And, 88957116c9 seems to introduce another restriction which we
> > > cannot create numa nodes on the machine. I got the following
> > > message when I set the numa config and started the VM:
> > > 
> > > 2020-10-02T00:31:46.780374Z qemu-system-x86_64: '-machine 
> > > memory-backend' and '-numa memdev' properties are mutually exclusive
> > 
> > It seems that this isn't a restriction for virtiofs. It's a bug introduced
> > by commit: 88957116c9.  element doesn't work regardless of virtiofs 
> > config...
> 
> Do you have domain XML that is failing?

Yes, I got the issue while virsh start:

  # virsh start Test
  error: Failed to start domain Test
  error: internal error: qemu unexpectedly closed the monitor: 
2020-10-05T17:31:21.078134Z qemu-system-x86_64: '-machine memory-backend' and 
'-numa memdev' properties are mutually exclusive

  # 

Here is the domain XML. I'm using libvirt with the head commit: 7d77fdb90f,
and qemu with the head commit: 36d9c2883e.
=

  Test
  12785e5c-542e-4896-87f3-2e068fb7107f
  8388608
  8388608
  4
  
hvm
  
  





  
  

  

  
  



  
  destroy
  restart
  destroy
  
/usr/local/bin/qemu-system-x86_64

  
  
  
  
  



  
  
  


  
  
  


  
  
  


  
  
  


  
  
  


  
  
  


  


  


  


  
  
  
  


  

  


  


  
  




  


  /dev/urandom
  

  

=

Thanks,
Masa



[PATCHv3 3/5] qemu: add schema 'fmode' and 'dmode' options

2020-10-05 Thread Brian Turek
Adds schema to validate the 'fmode' and 'dmode' attributes on a
'fileystem' node.  Checks to ensure that the values are 1-4 octal
digits long.

Signed-off-by: Brian Turek 
---
 docs/schemas/domaincommon.rng | 16 
 1 file changed, 16 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 316d93fb69..6c814d600a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -26,6 +26,12 @@
 
   
 
+  
+
+  [0-7]{1,4}
+
+  
+
   
@@ -2736,6 +2742,16 @@
 
   
 
+
+  
+
+  
+
+
+  
+
+  
+
 
   
 
-- 
2.25.1



[libvirt PATCH 1/9] spec: Simplify setting features off by default

2020-10-05 Thread Andrea Bolognani
The right-hand side of these expressions will always evaluate to
zero. Stop obfuscating this fact.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index d13aae5cf5..815ab246e9 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -85,15 +85,15 @@
 %endif
 
 # A few optional bits off by default, we enable later
-%define with_fuse  0%{!?_without_fuse:0}
-%define with_sanlock   0%{!?_without_sanlock:0}
-%define with_numad 0%{!?_without_numad:0}
-%define with_firewalld 0%{!?_without_firewalld:0}
-%define with_firewalld_zone 0%{!?_without_firewalld_zone:0}
-%define with_libssh2   0%{!?_without_libssh2:0}
-%define with_wireshark 0%{!?_without_wireshark:0}
-%define with_libssh0%{!?_without_libssh:0}
-%define with_bash_completion  0%{!?_without_bash_completion:0}
+%define with_fuse 0
+%define with_sanlock  0
+%define with_numad0
+%define with_firewalld0
+%define with_firewalld_zone   0
+%define with_libssh2  0
+%define with_wireshark0
+%define with_libssh   0
+%define with_bash_completion  0
 
 # Finally set the OS / architecture specific special cases
 
-- 
2.26.2



[libvirt PATCH 2/9] spec: firewalld is always enabled

2020-10-05 Thread Andrea Bolognani
Knowing this, we can remove some code.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 815ab246e9..4572044d2d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -88,7 +88,6 @@
 %define with_fuse 0
 %define with_sanlock  0
 %define with_numad0
-%define with_firewalld0
 %define with_firewalld_zone   0
 %define with_libssh2  0
 %define with_wireshark0
@@ -138,8 +137,6 @@
 %endif
 %endif
 
-%define with_firewalld 1
-
 %if 0%{?fedora} || 0%{?rhel} > 7
 %define with_firewalld_zone 0%{!?_without_firewalld_zone:1}
 %endif
@@ -1088,12 +1085,6 @@ exit 1
 %define arg_sanlock -Dsanlock=disabled
 %endif
 
-%if %{with_firewalld}
-%define arg_firewalld -Dfirewalld=enabled
-%else
-%define arg_firewalld -Dfirewalld=disabled
-%endif
-
 %if %{with_firewalld_zone}
 %define arg_firewalld_zone -Dfirewalld_zone=enabled
 %else
@@ -1169,7 +1160,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' 
%{_specdir}/%{name}.spec)
-Dlibpcap=enabled \
-Daudit=enabled \
-Ddtrace=enabled \
-   %{?arg_firewalld} \
+   -Dfirewalld=enabled \
%{?arg_firewalld_zone} \
%{?arg_wireshark} \
-Dpm_utils=disabled \
-- 
2.26.2



[libvirt PATCH 5/9] spec: Introduce with_dmidecode

2020-10-05 Thread Andrea Bolognani
To keep things maintainable, we want to have architecture handling
all in one spot instead of sprinkling %ifarch conditionals all over
the place.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 65df7dc79f..2401404008 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -94,6 +94,7 @@
 %define with_libssh2  0
 %define with_wireshark0
 %define with_libssh   0
+%define with_dmidecode0
 
 # Finally set the OS / architecture specific special cases
 
@@ -183,6 +184,10 @@
 %endif
 %endif
 
+%ifarch %{ix86} x86_64
+%define with_dmidecode 0%{!?_without_dmidecode:1}
+%endif
+
 # Force QEMU to run as non-root
 %define qemu_user  qemu
 %define qemu_group  qemu
@@ -434,7 +439,7 @@ Requires: iproute-tc
 %endif
 
 Requires: polkit >= 0.112
-%ifarch %{ix86} x86_64
+%if %{with_dmidecode}
 # For virConnectGetSysinfo
 Requires: dmidecode
 %endif
-- 
2.26.2



[libvirt PATCH 7/9] spec: Drop s390 architecture from conditionals

2020-10-05 Thread Andrea Bolognani
Neither Fedora nor RHEL build packages on this architecture.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index d8f689e651..e036307d30 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -109,12 +109,12 @@
 %endif
 
 # Numactl is not available on many non-x86 archs
-%ifarch s390 s390x %{arm} riscv64
+%ifarch s390x %{arm} riscv64
 %define with_numactl 0
 %endif
 
 # zfs-fuse is not available on some architectures
-%ifarch s390 s390x aarch64 riscv64
+%ifarch s390x aarch64 riscv64
 %define with_storage_zfs 0
 %endif
 
@@ -179,7 +179,7 @@
 %if %{with_qemu} || %{with_lxc}
 # numad is used to manage the CPU and memory placement dynamically,
 # it's not available on many non-x86 architectures.
-%ifnarch s390 s390x %{arm} riscv64
+%ifnarch s390x %{arm} riscv64
 %define with_numad0%{!?_without_numad:1}
 %endif
 %endif
-- 
2.26.2



[libvirt PATCHv3 0/4] Add support for QEMU's fmode and dmode

2020-10-05 Thread Brian Turek
Apologies for the second submission here. I got a kickback on two of the
emails saying it was "rejected due to security policies."

This third version of the patches fixes a bug where QEMU interpreted the
command line value passed to it as base-10 rather than base-8.  This new
version ensures there is always a preceeding 0 in the QEMU args (using
%04o formatting) and explictly sets it in the generated XML.

Brian Turek (4):
  qemu: capabilities: add QEMU_CAPS_FSDEV_CREATEMODE
  qemu: add support for 'fmode' and 'dmode' options
  qemu: add schema 'fmode' and 'dmode' options
  qemu: add docs for 'fmode' and 'dmode' options

 docs/formatdomain.rst | 12 
 docs/schemas/domaincommon.rng | 16 +
 src/conf/domain_conf.c| 27 
 src/conf/domain_conf.h|  2 +
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_command.c   |  6 ++
 src/qemu/qemu_validate.c  | 18 ++
 .../caps_2.10.0.aarch64.xml   |  1 +
 .../caps_2.10.0.ppc64.xml |  1 +
 .../caps_2.10.0.s390x.xml |  1 +
 .../caps_2.10.0.x86_64.xml|  1 +
 .../caps_2.11.0.s390x.xml |  1 +
 .../caps_2.11.0.x86_64.xml|  1 +
 .../caps_2.12.0.aarch64.xml   |  1 +
 .../caps_2.12.0.ppc64.xml |  1 +
 .../caps_2.12.0.s390x.xml |  1 +
 .../caps_2.12.0.x86_64.xml|  1 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 +
 .../caps_3.0.0.riscv32.xml|  1 +
 .../caps_3.0.0.riscv64.xml|  1 +
 .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 +
 .../caps_3.0.0.x86_64.xml |  1 +
 .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 +
 .../caps_3.1.0.x86_64.xml |  1 +
 .../caps_4.0.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |  1 +
 .../caps_4.0.0.riscv32.xml|  1 +
 .../caps_4.0.0.riscv64.xml|  1 +
 .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  1 +
 .../caps_4.0.0.x86_64.xml |  1 +
 .../caps_4.1.0.x86_64.xml |  1 +
 .../caps_4.2.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |  1 +
 .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  1 +
 .../caps_4.2.0.x86_64.xml |  1 +
 .../caps_5.0.0.aarch64.xml|  1 +
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  1 +
 .../caps_5.0.0.riscv64.xml|  1 +
 .../caps_5.0.0.x86_64.xml |  1 +
 .../caps_5.1.0.x86_64.xml |  1 +
 .../caps_5.2.0.x86_64.xml |  1 +
 .../virtio-9p-createmode.x86_64-latest.args   | 45 ++
 .../qemuxml2argvdata/virtio-9p-createmode.xml | 58 ++
 .../virtio-9p-createmode.x86_64-latest.xml| 61 +++
 tests/qemuxml2xmltest.c   |  1 +
 46 files changed, 283 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/virtio-9p-createmode.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.xml
 create mode 100644 
tests/qemuxml2xmloutdata/virtio-9p-createmode.x86_64-latest.xml

-- 
2.25.1



  1   2   >