Re: [libvirt] [PATCH] Add overrides for network port UUID getter/lookup methods

[Fabiano Fidêncio]

[snip]

> +static PyObject *
> +libvirt_virNetworkPortLookupByUUID(PyObject *self ATTRIBUTE_UNUSED,
> +   PyObject *args)
> +{
> +virNetworkPortPtr c_retval;
> +virNetworkPtr net;
> +PyObject *pyobj_net;
> +unsigned char *uuid;
> +int len;
> +
> +if (!PyArg_ParseTuple(args, (char *)"Oz#:virNetworkPortLookupByUUID",
> +  _net, , ))
> +return NULL;
> +net = (virNetworkPtr) PyvirNetwork_Get(pyobj_net);
> +

Shouldn't we also check whether net is NULL here?

> +if ((uuid == NULL) || (len != VIR_UUID_BUFLEN))
> +return VIR_PY_NONE;
> +
> +LIBVIRT_BEGIN_ALLOW_THREADS;
> +c_retval = virNetworkPortLookupByUUID(net, uuid);
> +LIBVIRT_END_ALLOW_THREADS;
> +
> +return libvirt_virNetworkPortPtrWrap((virNetworkPortPtr) c_retval);
> +}
> +

[snip]

With that fixed, Reviewed-by: Fabiano Fidêncio 


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

Re: [libvirt] [PATCH 13/23] src: replace last_component() with g_path_get_basename()

[Fabiano Fidêncio]

[snip]

> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index f072afe857..16a527e399 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -55,7 +55,6 @@
>  #include "virprobe.h"
>  #include "virprocess.h"
>  #include "virstring.h"
> -#include "dirname.h"
>  #include "passfd.h"
>
>  #if WITH_SSH2
> @@ -668,7 +667,7 @@ int virNetSocketNewConnectUNIX(const char *path,
>  remoteAddr.len = sizeof(remoteAddr.data.un);
>
>  if (spawnDaemon) {
> -const char *binname;
> +g_autofree char *binname = NULL;
>
>  if (spawnDaemon && !binary) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -677,7 +676,7 @@ int virNetSocketNewConnectUNIX(const char *path,
>  goto cleanup;
>  }
>
> -if (!(binname = last_component(binary)) || binname[0] == '\0') {
> +if (!(binname = g_path_get_basename(binary)) || binname[0] == '\0') {

IIUC, this check is no longer valid.
According to the g_path_get_basename() documentation "If file_name
ends with a directory separator it gets the component before the last
slash. If file_name consists only of directory separators (and on
Windows, possibly a drive letter), a single separator is returned. If
file_name is empty, it gets "."."

Knowing that, shouldn't we adapt the check accordingly?

[snip]

> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> index cd52a91ffd..c2499c0a20 100644
> --- a/src/util/virmdev.c
> +++ b/src/util/virmdev.c
> @@ -18,7 +18,6 @@
>
>  #include 
>
> -#include "dirname.h"
>  #include "virmdev.h"
>  #include "virlog.h"
>  #include "virerror.h"
> @@ -207,6 +206,7 @@ char *
>  virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
>  {
>  g_autofree char *result_path = NULL;
> +g_autofree char *result_file = NULL;
>  g_autofree char *iommu_path = NULL;
>  g_autofree char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr);
>  char *vfio_path = NULL;
> @@ -226,7 +226,9 @@ virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
>  return NULL;
>  }
>
> -vfio_path = g_strdup_printf("/dev/vfio/%s", last_component(result_path));
> +result_file = g_path_get_basename(result_path);
> +
> +vfio_path = g_strdup_printf("/dev/vfio/%s", result_file);

Please, while changing it, use g_build_filename() instead of g_strdup_printf().
[snip]


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

Re: [libvirt] [PATCH 19/23] util: replace gethostname() with g_get_hostname()

[Fabiano Fidêncio]

[snip]

> -r = gethostname(virLogHostname, sizeof(virLogHostname));
> -if (r == -1) {
> -ignore_value(virStrcpyStatic(virLogHostname, "(unknown)"));
> -} else {
> -NUL_TERMINATE(virLogHostname);
> -}
> +(void)g_get_host_name();

Why not use ignore_return() here?

[snip]


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

Re: [libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining

[Fabiano Fidêncio]

Daniel,

On Thu, Jan 2, 2020 at 3:59 PM Daniel P. Berrangé  wrote:
>
> In the last days before the xmas break I took some time to
> eliminate 14 more gnulib modules, bringing us down to just
> 50 left to go. They're getting harder to eliminate as we go
> on, but to give some hints, I've annotated every module in
> bootstrap.conf with a suggested replacement strategy.
>
> Daniel P. Berrangé (23):
>   build: set min version for CLang to 3.4 / XCode CLang to 5.1
>   docs: expand macOS platform support coverage
>   travis: add macOS Xcode 11.3 testing
>   util: add note about event file descriptors on Windows
>   src: always pull in glib/gstdio.h header
>   src: switch to use g_setenv/g_unsetenv

In the patch above I'd also take the opportunity and use TRUE/FALSE
instead of 1/0 in g_setenv().
Feel free to ignore as this is just a minor nitpick;

>   util: add compat wrapper for g_fsync
>   src: use g_fsync for portability
>   util: introduce virFileDataSync

What would be the impact of using g_fsync() on Linuxes as well?

>   src: use g_lstat() instead of lstat()
>   src: switch from fnmatch to g_pattern_match_simple

This one worries me a little bit about possible breakages, mainly
related to wildcards used in libvirtd.conf.

>   src: replace clock_gettime()/gettimeofday() with g_get_real_time()
>   src: replace last_component() with g_path_get_basename()

Please, take a look at this patch's reply.

>   util: replace IS_ABSOLUTE_FILE_NAME with g_path_is_absolute
>   src: replace mdir_name() with g_path_get_dirname()
>   src: remove unused imports of dirname.h
>   src: replace getcwd() with g_get_current_dir()
>   util: use realpath/g_canonicalize_filename
>   util: replace gethostname() with g_get_hostname()

Please, take a look at this patch's reply.

>   src: replace WSAStartup with g_networking_init()
>   src: replace strptime()/timegm()/mktime() with GDateTime APIs set
>   bootstrap: remove now unused gnulib modules
>   bootstrap: annotate with info about desired replacement
>

All the comments are more suggestions than blockers. The questions are
not blockers in any way.
Knowing that, for the series:

Reviewed-by: Fabiano Fidêncio 

[snip]


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

Re: [libvirt] [PATCH] qemu: Warn of restore with managed save being risky

[Michael Weiser]

Hello Daniel,

On Thu, Jan 02, 2020 at 11:54:23AM -0300, Daniel Henrique Barboza wrote:

> On 1/2/20 11:36 AM, Michael Weiser wrote:

> > I'd rather not reference virsh command options in the error message as
> > it would be highly confusing in any other context. For example, python
> > clients get the error message wrapped in an exception, augmented already
> > by a prefix telling them they need to force the operation:
> Good point.
> > 
> > Traceback (most recent call last):
> >File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in 
> > cb_wrapper
> >  callback(asyncjob, *args, **kwargs)
> >File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in 
> > tmpcb
> >  callback(*args, **kwargs)
> >File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line 
> > 66, in newfn
> >  ret = fn(self, *args, **kwargs)
> >File "/usr/share/virt-manager/virtManager/object/domain.py", line 1055, 
> > in revert_to_snapshot
> >  self._backend.revertToSnapshot(snap.get_backend())
> >File "/usr/lib64/python3.6/site-packages/libvirt.py", line 2088, in 
> > revertToSnapshot
> >  if ret == -1: raise libvirtError ('virDomainRevertToSnapshot() 
> > failed', dom=self)
> > libvirt.libvirtError: revert requires force: revert to snapshot while
> > there is a managed saved state will cause corruption when run, remove
> > saved state first
> > 
> > The same is actually the case for virsh already:
> > 
> > virsh # snapshot-revert debian --snapshotname snapshot1
> > error: revert requires force: revert to snapshot while there is a
> > managed saved state will cause corruption when run, remove saved state
> > first
> > 
> > virsh #
> > 
> > We could of course reword to better take context and prefix into
> > account, e.g.:
> Since there is already a "revert requires force" prefix in both python and
> virsh error messages, changing the error message of this v1 becomes more of
> a wording/flavor issue.

> > 
> > error: revert requires force: Removal of existing managed saved state
> > strongly recommended to avoid corruption

> I prefer this wording though :)

I went with:

error: revert requires force: snapshot without memory state, removal of
existing managed saved state strongly recommended to avoid corruption

because it gives the reason and recommended action in compact form.

v2 is on its way.
-- 
Michael


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

[libvirt] [PATCH v2 1/3] qemu: Warn of restore with managed save being risky

[Michael Weiser]

Internal snapshots of a non-running domain do not carry any memory state
and restoring such a snapshot will not replace existing saved memory
state. This allows a scenario, where a user first suspends a domain into
managedsave, restores a non-running snapshot and then resumes the domain
from managedsave. After that, the guest system will run with its
previous memory state atop a different disk state. The most obvious
possible fallout from this is extensive file system corruption. Swap
content and RAID bitmaps might also be off.

This has been discussed[1] and fixed[2] from the end-user perspective for
virt-manager.

This patch marks the restore operation as risky at the libvirt level,
requiring the user to remove the saved memory state first or force the
operation.

[1] https://www.redhat.com/archives/virt-tools-list/2019-November/msg00011.html
[2] https://www.redhat.com/archives/virt-tools-list/2019-December/msg00049.html

Signed-off-by: Michael Weiser 
Reviewed-by: Daniel Henrique Barboza 
Cc: Cole Robinson 
---
 src/qemu/qemu_driver.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec8faf384c..691a9b45c7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16652,6 +16652,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
_("must respawn qemu to start inactive snapshot"));
 goto endjob;
 }
+if (vm->hasManagedSave &&
+!(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
+  snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
+virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
+   _("snapshot without memory state, removal of "
+ "existing managed saved state strongly "
+ "recommended to avoid corruption"));
+goto endjob;
+}
 }
 
 if (snap->def->dom) {
-- 
2.24.1


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

[libvirt] [PATCH v2 0/3] Mark and document restore with managed save as risky

[Michael Weiser]

This series marks restore of an inactive qemu snapshot while there is
managed saved state as risky due to the reasons explained in patch 1 and
3. Patch 2 is a simple reformatting of the documentation with no other
changes in preparation of addition of more reasons why reverts might
need to be forced.

Changes from v1:
- reword error message to "error: revert requires force: snapshot
  without memory state, removal of existing managed saved state strongly
  recommended to avoid corruption"
- add documentation of the new behaviour

Michael Weiser (3):
  qemu: Warn of restore with managed save being risky
  docs: Reformat snapshot-revert force reasons
  docs: Add snapshot-revert qemu managedsave force

 docs/manpages/virsh.rst | 39 ---
 src/qemu/qemu_driver.c  |  9 +
 2 files changed, 33 insertions(+), 15 deletions(-)

-- 
2.24.1


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

Re: [libvirt] [PATCH RFC] update cacrl without restarting libvirtd via virt-admin

[Daniel P . Berrangé]

On Sat, Dec 28, 2019 at 02:18:20AM +, Zhangbo (Oscar) wrote:
> This is an RFC request for supporting virt-admin to update cacrl without
> restarting libvirtd.
> 
> When a client wants to establish a TLS connection with libvirtd, a CRL
> file is used by libvirtd to verify the client's certificate. Right now,
> if the CRL file is changed, you must restart libvirtd to make it take
> effect. The restart behavior of libvirtd will cause clients connecting
> with libvirtd to fail.
> 
> In a server cluster, the CRL file may be updated quite frequently due to
> the large amount of certificates.  If the new CRL does not take effect
> in time, there are security risks. So you may need to restart libvirtd
> frequently to make the CRL take effect in time. However, frequent restarts
> will affect the reliability of cluster virtual machine management(such as
> openstack) services.
> 
> This RFC patch adds a virt-admin command to update the server's CRL *online*.
> 
> This patch is not elegant enough, if this feature makes sense, I'd do more
> improvements.

I agree that not being able to update the CRL without restarts is a
significant problem that needs a fix. I'd suggest it is just part of
an even bigger problem - we can't update the CA cert, server cert / key
either. This is increasingly important as the popularity of short-expiry
serve certs increases.

So I think we should make the command be able to update all these TLS
related PEM files. eg have a more general command

 "virt-admin daemon-reload-tls"

to update CA cert, CA crl, server cert+key.  The impl could check the
timestamps on the individual PEM files, so it avoids reloading the
files which haven't changed since last time.

> 
> ---
> include/libvirt/libvirt-admin.h  |  4 ++
> src/admin/admin_protocol.x   | 13 +-
> src/admin/admin_server.c | 13 ++
> src/admin/admin_server.h |  4 ++
> src/admin/libvirt-admin.c  | 33 
> src/admin/libvirt_admin_private.syms   |  1 +
> src/admin/libvirt_admin_public.syms|  1 +
> src/rpc/virnetserver.c | 58 +++
> src/rpc/virnetserver.h |  3 ++
> src/rpc/virnettlscontext.c  | 33 
> src/rpc/virnettlscontext.h  |  3 ++
> tools/virt-admin.c| 59 

docs/manpages/virt-admin.rst will need an update too.



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

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

[libvirt] [PATCH v2 2/3] docs: Reformat snapshot-revert force reasons

[Michael Weiser]

Reformat explanations of the snapshot-revert force reasons in
preparation for more to be added. This is a simple reformat without any
wording changes.

Signed-off-by: Michael Weiser 
Cc: Cole Robinson 
---
 docs/manpages/virsh.rst | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..f5f962cba1 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6952,20 +6952,22 @@ transient domains cannot be inactive, it is required to 
use one of these
 flags when reverting to a disk snapshot of a transient domain.
 
 There are two cases where a snapshot revert involves extra risk, which
-requires the use of *--force* to proceed.  One is the case of a
-snapshot that lacks full domain information for reverting
-configuration (such as snapshots created prior to libvirt 0.9.5);
-since libvirt cannot prove that the current configuration matches what
-was in use at the time of the snapshot, supplying *--force* assures
-libvirt that the snapshot is compatible with the current configuration
-(and if it is not, the domain will likely fail to run).  The other is
-the case of reverting from a running domain to an active state where a
-new hypervisor has to be created rather than reusing the existing
-hypervisor, because it implies drawbacks such as breaking any existing
-VNC or Spice connections; this condition happens with an active
-snapshot that uses a provably incompatible configuration, as well as
-with an inactive snapshot that is combined with the *--start* or
-*--pause* flag.
+requires the use of *--force* to proceed:
+
+  * One is the case of a snapshot that lacks full domain information for
+reverting configuration (such as snapshots created prior to libvirt
+0.9.5); since libvirt cannot prove that the current configuration matches
+what was in use at the time of the snapshot, supplying *--force* assures
+libvirt that the snapshot is compatible with the current configuration
+(and if it is not, the domain will likely fail to run).
+
+  * The other is the case of reverting from a running domain to an active
+state where a new hypervisor has to be created rather than reusing the
+existing hypervisor, because it implies drawbacks such as breaking any
+existing VNC or Spice connections; this condition happens with an active
+snapshot that uses a provably incompatible configuration, as well as with
+an inactive snapshot that is combined with the *--start* or *--pause*
+flag.
 
 
 snapshot-delete
-- 
2.24.1


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

[libvirt] [PATCH v2 3/3] docs: Add snapshot-revert qemu managedsave force

[Michael Weiser]

Add documentation for additional reason why snapshot-revert might need
to be forced. This explains why restoring an inactive snapshot while
there is managed saved state is refused by default.

Signed-off-by: Michael Weiser 
Cc: Cole Robinson 
---
 docs/manpages/virsh.rst | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index f5f962cba1..09be872fdf 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6951,7 +6951,7 @@ no vm state leaves the domain in an inactive state.  
Passing either the
 transient domains cannot be inactive, it is required to use one of these
 flags when reverting to a disk snapshot of a transient domain.
 
-There are two cases where a snapshot revert involves extra risk, which
+There are a number of cases where a snapshot revert involves extra risk, which
 requires the use of *--force* to proceed:
 
   * One is the case of a snapshot that lacks full domain information for
@@ -6961,7 +6961,7 @@ requires the use of *--force* to proceed:
 libvirt that the snapshot is compatible with the current configuration
 (and if it is not, the domain will likely fail to run).
 
-  * The other is the case of reverting from a running domain to an active
+  * Another is the case of reverting from a running domain to an active
 state where a new hypervisor has to be created rather than reusing the
 existing hypervisor, because it implies drawbacks such as breaking any
 existing VNC or Spice connections; this condition happens with an active
@@ -6969,6 +6969,13 @@ requires the use of *--force* to proceed:
 an inactive snapshot that is combined with the *--start* or *--pause*
 flag.
 
+  * Also, libvirt will refuse to restore snapshots of inactive qemu domains
+while there is managed saved state. This is because those snapshots do not
+contain memory state and will therefore not replace the exising memory
+state. This ends up switching a disk underneath a running system and will
+likely cause extensive filesystem corruption or crashes due to swap content
+mismatches when run.
+
 
 snapshot-delete
 ---
-- 
2.24.1


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

Re: [libvirt] [PATCH] fix class type instantiated when listing network ports

[Erik Skultety]

On Thu, Jan 02, 2020 at 03:47:31PM +, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  libvirt-override-virNetwork.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libvirt-override-virNetwork.py b/libvirt-override-virNetwork.py
> index 749ee44..4245322 100644
> --- a/libvirt-override-virNetwork.py
> +++ b/libvirt-override-virNetwork.py
> @@ -6,6 +6,6 @@
>
>  retlist = list()
>  for domptr in ret:
> -retlist.append(virNetwork(self, _obj=domptr))
> +retlist.append(virNetworkPort(self, _obj=domptr))

The doc string deserves an update as well.
Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH] fix class type instantiated when listing network ports

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 libvirt-override-virNetwork.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-override-virNetwork.py b/libvirt-override-virNetwork.py
index 749ee44..4245322 100644
--- a/libvirt-override-virNetwork.py
+++ b/libvirt-override-virNetwork.py
@@ -6,6 +6,6 @@
 
 retlist = list()
 for domptr in ret:
-retlist.append(virNetwork(self, _obj=domptr))
+retlist.append(virNetworkPort(self, _obj=domptr))
 
 return retlist
-- 
2.24.1

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

[libvirt] [PATCH] Add overrides for network port UUID getter/lookup methods

[Daniel P . Berrangé]

The generator creates broken code for all these methods.

Signed-off-by: Daniel P. Berrangé 
---
 generator.py |  3 ++
 libvirt-override-api.xml | 16 
 libvirt-override.c   | 83 
 3 files changed, 102 insertions(+)

diff --git a/generator.py b/generator.py
index cba9d47..426f007 100755
--- a/generator.py
+++ b/generator.py
@@ -430,6 +430,9 @@ skip_impl = (
 'virNetworkGetUUID',
 'virNetworkGetUUIDString',
 'virNetworkLookupByUUID',
+'virNetworkPortGetUUID',
+'virNetworkPortGetUUIDString',
+'virNetworkPortLookupByUUID',
 'virDomainGetAutostart',
 'virNetworkGetAutostart',
 'virDomainBlockStats',
diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
index 7a0d4c5..4ab403c 100644
--- a/libvirt-override-api.xml
+++ b/libvirt-override-api.xml
@@ -64,6 +64,12 @@
   
   
 
+
+  Try to lookup a port on the given network based on its UUID.
+  
+  
+  
+
 
   Extract information about a domain. Note that if the connection 
used to get the domain is limited only a partial set of the information can be 
extracted.
   
@@ -153,6 +159,16 @@
   
   
 
+
+  Extract the UUID unique Identifier of a network port.
+  
+  
+
+
+  Fetch globally unique ID of the network port as a string.
+  
+  
+
 
   Extract the UUID unique Identifier of a storage pool.
   
diff --git a/libvirt-override.c b/libvirt-override.c
index 4e4c00a..2b39ace 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -10185,6 +10185,86 @@ libvirt_virNetworkPortGetParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 virTypedParamsFree(params, nparams);
 return dict;
 }
+
+static PyObject *
+libvirt_virNetworkPortGetUUID(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+unsigned char uuid[VIR_UUID_BUFLEN];
+virNetworkPortPtr port;
+PyObject *pyobj_port;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char *)"O:virNetworkPortGetUUID", 
_port))
+return NULL;
+port = (virNetworkPortPtr) PyvirNetworkPort_Get(pyobj_port);
+
+if (port == NULL)
+return VIR_PY_NONE;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virNetworkPortGetUUID(port, [0]);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_retval < 0)
+return VIR_PY_NONE;
+
+return libvirt_charPtrSizeWrap((char *) [0], VIR_UUID_BUFLEN);
+}
+
+static PyObject *
+libvirt_virNetworkPortGetUUIDString(PyObject *self ATTRIBUTE_UNUSED,
+PyObject *args)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virNetworkPortPtr port;
+PyObject *pyobj_port;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char *)"O:virNetworkPortGetUUIDString",
+  _port))
+return NULL;
+port = (virNetworkPortPtr) PyvirNetworkPort_Get(pyobj_port);
+
+if (port == NULL)
+return VIR_PY_NONE;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virNetworkPortGetUUIDString(port, [0]);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_retval < 0)
+return VIR_PY_NONE;
+
+return libvirt_constcharPtrWrap((char *) [0]);
+}
+
+static PyObject *
+libvirt_virNetworkPortLookupByUUID(PyObject *self ATTRIBUTE_UNUSED,
+   PyObject *args)
+{
+virNetworkPortPtr c_retval;
+virNetworkPtr net;
+PyObject *pyobj_net;
+unsigned char *uuid;
+int len;
+
+if (!PyArg_ParseTuple(args, (char *)"Oz#:virNetworkPortLookupByUUID",
+  _net, , ))
+return NULL;
+net = (virNetworkPtr) PyvirNetwork_Get(pyobj_net);
+
+if ((uuid == NULL) || (len != VIR_UUID_BUFLEN))
+return VIR_PY_NONE;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virNetworkPortLookupByUUID(net, uuid);
+LIBVIRT_END_ALLOW_THREADS;
+
+return libvirt_virNetworkPortPtrWrap((virNetworkPortPtr) c_retval);
+}
+
+
 #endif /* LIBVIR_CHECK_VERSION(5, 5, 0) */
 
 #if LIBVIR_CHECK_VERSION(5, 7, 0)
@@ -10535,6 +10615,9 @@ static PyMethodDef libvirtMethods[] = {
 {(char *) "virNetworkListAllPorts", libvirt_virNetworkListAllPorts, 
METH_VARARGS, NULL},
 {(char *) "virNetworkPortSetParameters", 
libvirt_virNetworkPortSetParameters, METH_VARARGS, NULL},
 {(char *) "virNetworkPortGetParameters", 
libvirt_virNetworkPortGetParameters, METH_VARARGS, NULL},
+{(char *) "virNetworkPortGetUUID", libvirt_virNetworkPortGetUUID, 
METH_VARARGS, NULL},
+{(char *) "virNetworkPortGetUUIDString", 
libvirt_virNetworkPortGetUUIDString, METH_VARARGS, NULL},
+{(char *) "virNetworkPortLookupByUUID", 
libvirt_virNetworkPortLookupByUUID, METH_VARARGS, NULL},
 #endif /* LIBVIR_CHECK_VERSION(5, 5, 0) */
 #if LIBVIR_CHECK_VERSION(5, 7, 0)
 {(char *) "virDomainGetGuestInfo", libvirt_virDomainGetGuestInfo, 
METH_VARARGS, NULL},
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com

[libvirt] [PATCH] virsh: Add a completer for `domifaddr` --source parameter.

[Julio Faracco]

The command `domifaddr` can use three different sources to grab IP
address of a Virtual Machine: lease, agent and arp. This parameter does
not have a completer function to return source options.

Signed-off-by: Julio Faracco 
---
 tools/virsh-completer-domain.c | 17 +
 tools/virsh-completer-domain.h |  5 +
 tools/virsh-domain-monitor.c   |  1 +
 3 files changed, 23 insertions(+)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 0311ee50d0..c8709baa38 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl,
 
 return virshCommaStringListComplete(mode, modes);
 }
+
+
+char **
+virshDomainIfAddrSourceCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags)
+{
+const char *sources[] = {"lease", "agent", "arp", NULL};
+const char *source = NULL;
+
+virCheckFlags(0, NULL);
+
+if (vshCommandOptStringQuiet(ctl, cmd, "source", ) < 0)
+return NULL;
+
+return virshCommaStringListComplete(source, sources);
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index 083ab327cc..f5e5625051 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -53,3 +53,8 @@ char ** virshDomainDeviceAliasCompleter(vshControl *ctl,
 char ** virshDomainShutdownModeCompleter(vshControl *ctl,
  const vshCmd *cmd,
  unsigned int flags);
+
+char **
+virshDomainIfAddrSourceCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 30b186ffd1..1d1f87eb9e 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2346,6 +2346,7 @@ static const vshCmdOptDef opts_domifaddr[] = {
 {.name = "source",
  .type = VSH_OT_STRING,
  .flags = VSH_OFLAG_NONE,
+ .completer = virshDomainIfAddrSourceCompleter,
  .help = N_("address source: 'lease', 'agent', or 'arp'")},
 {.name = NULL}
 };
-- 
2.20.1


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

[libvirt] [PATCH v2 3/5] util: Use g_auto in virFirewallStartTransaction()

[Fabiano Fidêncio]

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 2177617ecf..564e2fe0be 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -278,6 +278,7 @@ virFirewallGroupFree(virFirewallGroupPtr group)
 VIR_FREE(group);
 }
 
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallGroupPtr, virFirewallGroupFree, 
NULL);
 
 /**
  * virFirewallFree:
@@ -575,7 +576,7 @@ size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule)
 void virFirewallStartTransaction(virFirewallPtr firewall,
  unsigned int flags)
 {
-virFirewallGroupPtr group;
+g_auto(virFirewallGroupPtr) group = NULL;
 
 VIR_FIREWALL_RETURN_IF_ERROR(firewall);
 
@@ -586,10 +587,9 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
 
 if (VIR_EXPAND_N(firewall->groups,
  firewall->ngroups, 1) < 0) {
-virFirewallGroupFree(group);
 return;
 }
-firewall->groups[firewall->ngroups - 1] = group;
+firewall->groups[firewall->ngroups - 1] = g_steal_pointer();
 firewall->currentGroup = firewall->ngroups - 1;
 }
 
-- 
2.24.1

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

[libvirt] [PATCH v2 1/5] util: Don't set/check for ENOMEM as a firewall error

[Fabiano Fidêncio]

As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's not set nor check for ENOMEM firewall's error and
simplify the code whenever it's possible.

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 30 ++
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index ee72b579e4..6f7b5306e5 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -391,7 +391,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 return rule;
 
  no_memory:
-firewall->err = ENOMEM;
 virFirewallRuleFree(rule);
 return NULL;
 }
@@ -492,10 +491,8 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
 
 ADD_ARG(rule, arg);
 
-return;
-
  no_memory:
-firewall->err = ENOMEM;
+return;
 }
 
 
@@ -514,10 +511,8 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
 
 ADD_ARG(rule, arg);
 
-return;
-
  no_memory:
-firewall->err = ENOMEM;
+return;
 }
 
 
@@ -532,10 +527,8 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
 args++;
 }
 
-return;
-
  no_memory:
-firewall->err = ENOMEM;
+return;
 }
 
 
@@ -553,12 +546,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
 while ((str = va_arg(list, char *)) != NULL)
 ADD_ARG(rule, str);
 
-va_end(list);
-
-return;
-
  no_memory:
-firewall->err = ENOMEM;
 va_end(list);
 }
 
@@ -591,15 +579,13 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
 
 VIR_FIREWALL_RETURN_IF_ERROR(firewall);
 
-if (!(group = virFirewallGroupNew())) {
-firewall->err = ENOMEM;
+if (!(group = virFirewallGroupNew()))
 return;
-}
+
 group->actionFlags = flags;
 
 if (VIR_EXPAND_N(firewall->groups,
  firewall->ngroups, 1) < 0) {
-firewall->err = ENOMEM;
 virFirewallGroupFree(group);
 return;
 }
@@ -747,10 +733,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
 if (rule->queryCB(firewall, rule->layer, (const char *const *)lines, 
rule->queryOpaque) < 0)
 return -1;
 
-if (firewall->err == ENOMEM) {
-virReportOOMError();
-return -1;
-}
 if (firewall->err) {
 virReportSystemError(firewall->err, "%s",
  _("Unable to create rule"));
@@ -818,7 +800,7 @@ virFirewallApply(virFirewallPtr firewall)
_("Failed to initialize a valid firewall backend"));
 goto cleanup;
 }
-if (!firewall || firewall->err == ENOMEM) {
+if (!firewall) {
 virReportOOMError();
 goto cleanup;
 }
-- 
2.24.1

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

[libvirt] [PATCH v2 5/5] docs: Remove mention to no_memory label

[Fabiano Fidêncio]

no_memory labels have been entirely removed from libvirt code base.
Knowing that, let's also remove any mention to the label from our
hacking guide.

Signed-off-by: Fabiano Fidêncio 
---
 docs/hacking.html.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 74aba5d46b..90bd0ddc81 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1491,7 +1491,6 @@ BAD:
 
   error: A path only taken upon return with an error code
 cleanup: A path taken upon return with success code + optional error
-  no_memory: A path only taken upon return with an OOM error code
   retry: If needing to jump upwards (e.g., retry on EINTR)
 
 
-- 
2.24.1

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

[libvirt] [PATCH v2 4/5] util: Adapt ADD_ARG() macro

[Fabiano Fidêncio]

As VIR_RESIZE_N() macro already aborts in case of OOM, there's no reason
to check for its output in the ADD_ARG() macro.

By doing this, we can simply get rid of a all no_memory labels spread in
the virfirewall.c file.

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 564e2fe0be..7c8040880c 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -320,11 +320,9 @@ void virFirewallFree(virFirewallPtr firewall)
 
 #define ADD_ARG(rule, str) \
 do { \
-if (VIR_RESIZE_N(rule->args, \
- rule->argsAlloc, \
- rule->argsLen, 1) < 0) \
-goto no_memory; \
- \
+ignore_value(VIR_RESIZE_N(rule->args, \
+  rule->argsAlloc, \
+  rule->argsLen, 1)); \
 rule->args[rule->argsLen++] = g_strdup(str); \
 } while (0)
 
@@ -391,9 +389,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 
 
 return g_steal_pointer();
-
- no_memory:
-return NULL;
 }
 
 
@@ -491,9 +486,6 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
 VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
 
 ADD_ARG(rule, arg);
-
- no_memory:
-return;
 }
 
 
@@ -511,9 +503,6 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
 va_end(list);
 
 ADD_ARG(rule, arg);
-
- no_memory:
-return;
 }
 
 
@@ -527,9 +516,6 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
 ADD_ARG(rule, *args);
 args++;
 }
-
- no_memory:
-return;
 }
 
 
@@ -547,7 +533,6 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
 while ((str = va_arg(list, char *)) != NULL)
 ADD_ARG(rule, str);
 
- no_memory:
 va_end(list);
 }
 
-- 
2.24.1

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

[libvirt] [PATCH v2 2/5] util: Use g_auto/g_autofree in virFirewallAddRuleFullV()

[Fabiano Fidêncio]

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 6f7b5306e5..2177617ecf 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -257,6 +257,7 @@ virFirewallRuleFree(virFirewallRulePtr rule)
 VIR_FREE(rule);
 }
 
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallRulePtr, virFirewallRuleFree, NULL);
 
 static void
 virFirewallGroupFree(virFirewallGroupPtr group)
@@ -335,8 +336,8 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 va_list args)
 {
 virFirewallGroupPtr group;
-virFirewallRulePtr rule;
-char *str;
+g_auto(virFirewallRulePtr) rule = NULL;
+g_autofree char *str = NULL;
 
 VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall);
 
@@ -348,7 +349,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 
 
 if (VIR_ALLOC(rule) < 0)
-goto no_memory;
+return NULL;
 
 rule->layer = layer;
 rule->queryCB = cb;
@@ -379,19 +380,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 if (VIR_APPEND_ELEMENT_COPY(group->rollback,
 group->nrollback,
 rule) < 0)
-goto no_memory;
+return NULL;
 } else {
 if (VIR_APPEND_ELEMENT_COPY(group->action,
 group->naction,
 rule) < 0)
-goto no_memory;
+return NULL;
 }
 
 
-return rule;
+return g_steal_pointer();
 
  no_memory:
-virFirewallRuleFree(rule);
 return NULL;
 }
 
-- 
2.24.1

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

[libvirt] [PATCH v2 0/5] Remove the last "no_memory" label

[Fabiano Fidêncio]

This series touches virfirewall.c, the last place where a no_memory
label can be found.

The series:
- Gets rid of setting and checking for ENOMEM as a firewall's error;
- Use g_auto / g_autofree in a few different places;
- Adapt ADD_ARG() macro as VIR_RESIZE_N() just aborts in case of OOM;
- Remove no_memory mention from the hacking guide;

Changes since v1:
https://www.redhat.com/archives/libvir-list/2020-January/msg00018.html
- Daniel suggested to get rid of ADD_ARG(). Instead of doing so, I've
  adapted the macro (as it does more than just a realloc) and, by doing
  so, got rid of the no_memory labels;
- Dropped:
  - util: Rename 'no_memory' label to 'cleanup'
  - util: Add ADD_ARG_RETURN_ON_ERROR
  - util: Add ADD_ARG_RETURN_VALUE_ON_ERROR
- Added:
  - util: Adapt ADD_ARG() macro

Fabiano Fidêncio (5):
  util: Don't set/check for ENOMEM as a firewall error
  util: Use g_auto/g_autofree in virFirewallAddRuleFullV()
  util: Use g_auto in virFirewallStartTransaction()
  util: Adapt ADD_ARG() macro
  docs: Remove mention to no_memory label

 docs/hacking.html.in   |  1 -
 src/util/virfirewall.c | 65 +++---
 2 files changed, 16 insertions(+), 50 deletions(-)

-- 
2.24.1

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

[libvirt] [PATCH 14/23] util: replace IS_ABSOLUTE_FILE_NAME with g_path_is_absolute

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 src/qemu/qemu_domain.c | 6 ++
 src/util/virfile.c | 4 ++--
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 187aaf5737..f545d0b888 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -77,8 +77,6 @@
 # include 
 #endif
 
-#include "dosname.h"
-
 #define QEMU_QXL_VGAMEM_DEFAULT 16 * 1024
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
@@ -14340,7 +14338,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
  *   (want /dev/disk/by-id/../../sda)
  * /dev/stdout -> /proc/self/fd/1 (no change needed)
  */
-if (IS_RELATIVE_FILE_NAME(target)) {
+if (!g_path_is_absolute(target)) {
 char *c = NULL, *tmp = NULL, *devTmp = NULL;
 
 devTmp = g_strdup(device);
@@ -15312,7 +15310,7 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr 
driver,
 return ret;
 }
 
-if (IS_RELATIVE_FILE_NAME(target)) {
+if (!g_path_is_absolute(target)) {
 char *c = NULL, *tmp = NULL, *fileTmp = NULL;
 
 fileTmp = g_strdup(file);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 97f86a46e9..1ce909b033 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1562,7 +1562,7 @@ virFileResolveLinkHelper(const char *linkpath,
 /* We don't need the full canonicalization of intermediate
  * directories, if linkpath is absolute and the basename is
  * already a non-symlink.  */
-if (IS_ABSOLUTE_FILE_NAME(linkpath) && !intermediatePaths) {
+if (g_path_is_absolute(linkpath) && !intermediatePaths) {
 if (g_lstat(linkpath, ) < 0)
 return -1;
 
@@ -1640,7 +1640,7 @@ virFindFileInPath(const char *file)
 /* if we are passed an absolute path (starting with /), return a
  * copy of that path, after validating that it is executable
  */
-if (IS_ABSOLUTE_FILE_NAME(file)) {
+if (g_path_is_absolute(file)) {
 if (!virFileIsExecutable(file))
 return NULL;
 
-- 
2.24.1

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

[libvirt] [PATCH 11/23] src: switch from fnmatch to g_pattern_match_simple

[Daniel P . Berrangé]

The g_pattern_match function_simple is an acceptably close
approximation of fnmatch for libvirt's needs.

In contrast to fnmatch(), the '/' character can be matched
by the wildcards, there are no '[...]' character ranges and
'*' and '?' can not be escaped to include them literally in
a pattern.

Signed-off-by: Daniel P. Berrangé 
---
 src/qemu/qemu_firmware.c|  4 +---
 src/remote/libvirtd.conf.in |  8 ++--
 src/rpc/virnetsaslcontext.c | 11 +--
 src/rpc/virnettlscontext.c  | 10 +-
 src/util/virlog.c   |  5 ++---
 tests/virconfdata/libvirtd.conf |  8 ++--
 tests/virconfdata/libvirtd.out  |  8 ++--
 tools/virt-login-shell-helper.c |  5 ++---
 8 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index f62ce90ac9..6a76d355f5 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -20,8 +20,6 @@
 
 #include 
 
-#include 
-
 #include "qemu_firmware.h"
 #include "qemu_interop_config.h"
 #include "configmake.h"
@@ -921,7 +919,7 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
 continue;
 
 for (j = 0; j < fw->targets[i]->nmachines; j++) {
-if (fnmatch(fw->targets[i]->machines[j], machine, 0) == 0)
+if (g_pattern_match_simple(fw->targets[i]->machines[j], machine))
 return true;
 }
 }
diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in
index f984ce0478..34741183cc 100644
--- a/src/remote/libvirtd.conf.in
+++ b/src/remote/libvirtd.conf.in
@@ -262,7 +262,9 @@
 #
 #"C=GB,ST=London,L=London,O=Red Hat,CN=*"
 #
-# See the POSIX fnmatch function for the format of the wildcards.
+# See the g_pattern_match function for the format of the wildcards:
+#
+# https://developer.gnome.org/glib/stable/glib-Glob-style-pattern-matching.html
 #
 # NB If this is an empty list, no client can connect, so comment out
 # entirely rather than using empty list to disable these checks
@@ -288,7 +290,9 @@
 #
 #"*@EXAMPLE.COM"
 #
-# See the POSIX fnmatch function for the format of the wildcards.
+# See the g_pattern_match function for the format of the wildcards.
+#
+# https://developer.gnome.org/glib/stable/glib-Glob-style-pattern-matching.html
 #
 # NB If this is an empty list, no client can connect, so comment out
 # entirely rather than using empty list to disable these checks
diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c
index 01ff41b778..e7ed8f4390 100644
--- a/src/rpc/virnetsaslcontext.c
+++ b/src/rpc/virnetsaslcontext.c
@@ -20,8 +20,6 @@
 
 #include 
 
-#include 
-
 #include "virnetsaslcontext.h"
 #include "virnetmessage.h"
 
@@ -155,17 +153,10 @@ int virNetSASLContextCheckIdentity(virNetSASLContextPtr 
ctxt,
 }
 
 while (*wildcards) {
-int rv = fnmatch(*wildcards, identity, 0);
-if (rv == 0) {
+if (g_pattern_match_simple(*wildcards, identity)) {
 ret = 1;
 goto cleanup; /* Successful match */
 }
-if (rv != FNM_NOMATCH) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Malformed TLS whitelist regular expression 
'%s'"),
-   *wildcards);
-goto cleanup;
-}
 
 wildcards++;
 }
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 08944f6771..44f0dfce77 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -21,7 +21,6 @@
 #include 
 
 #include 
-#include 
 
 #include 
 #include 
@@ -361,15 +360,8 @@ virNetTLSContextCheckCertDNWhitelist(const char *dname,
  const char *const*wildcards)
 {
 while (*wildcards) {
-int ret = fnmatch(*wildcards, dname, 0);
-if (ret == 0) /* Successful match */
+if (g_pattern_match_simple(*wildcards, dname))
 return 1;
-if (ret != FNM_NOMATCH) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Malformed TLS whitelist regular expression 
'%s'"),
-   *wildcards);
-return -1;
-}
 
 wildcards++;
 }
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 6bae56e2e3..aa98024e1c 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -36,7 +36,6 @@
 #if HAVE_SYS_UN_H
 # include 
 #endif
-#include 
 
 #include "virerror.h"
 #include "virlog.h"
@@ -488,7 +487,7 @@ virLogSourceUpdate(virLogSourcePtr source)
 size_t i;
 
 for (i = 0; i < virLogNbFilters; i++) {
-if (fnmatch(virLogFilters[i]->match, source->name, 0) == 0) {
+if (g_pattern_match_simple(virLogFilters[i]->match, source->name)) 
{
 priority = virLogFilters[i]->priority;
 break;
 }
@@ -1338,7 +1337,7 @@ virLogFilterNew(const char *match,
 return NULL;
 }
 
-/* We must treat 'foo' as equiv to '*foo*' for fnmatch
+/* We 

[libvirt] [PATCH 23/23] bootstrap: annotate with info about desired replacement

[Daniel P . Berrangé]

Add a comment against each gnulib module suggesting strategy
for replacement.

Signed-off-by: Daniel P. Berrangé 
---
 bootstrap.conf | 159 +
 1 file changed, 107 insertions(+), 52 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 3d9243fa01..ae9ecb4039 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -17,58 +17,113 @@
 # .
 
 # gnulib modules used by this package.
-gnulib_modules='
-accept
-bind
-chown
-close
-connect
-configmake
-environ
-fcntl
-fcntl-h
-getaddrinfo
-getpass
-getpeername
-getsockname
-intprops
-ioctl
-largefile
-listen
-localeconv
-manywarnings
-mgetgroups
-net_if
-netdb
-nonblocking
-openpty
-passfd
-physmem
-pipe-posix
-pipe2
-poll
-posix-shell
-pthread_sigmask
-recv
-send
-setsockopt
-sigaction
-sigpipe
-socket
-stat-time
-strchrnul
-strtok_r
-sys_stat
-sys_wait
-termios
-time_r
-ttyname_r
-uname
-verify
-waitpid
-warnings
-wcwidth
-'
+
+# NB the GSocket conversion is non-trivial due to the
+# different FD vs HANDLE usage in gnulib vs glib. Need
+# to find a way to duplicate a socket HANDLE before
+# turning it into a FD, since closing an FD also closes
+# the original HANDLE.
+
+# -> GSocket
+gnulib_modules="$gnulib_modules accept"
+# -> GSocket
+gnulib_modules="$gnulib_modules bind"
+# -> conditional build to avoid Win32
+gnulib_modules="$gnulib_modules chown"
+# -> GSocket
+gnulib_modules="$gnulib_modules close"
+# -> GSocket
+gnulib_modules="$gnulib_modules connect"
+# -> Meson
+gnulib_modules="$gnulib_modules configmake"
+# -> eliminate usage in some manner
+gnulib_modules="$gnulib_modules environ"
+# -> GSocket
+gnulib_modules="$gnulib_modules fcntl"
+# -> conditional build avoid win32
+gnulib_modules="$gnulib_modules fcntl-h"
+# -> GSocket
+gnulib_modules="$gnulib_modules getaddrinfo"
+# -> copy gnuliub win32 impl
+gnulib_modules="$gnulib_modules getpass"
+# -> GSocket
+gnulib_modules="$gnulib_modules getpeername"
+# -> GSocket
+gnulib_modules="$gnulib_modules getsockname"
+# -> copy gnulib STRBUFLEN macro
+gnulib_modules="$gnulib_modules intprops"
+# -> GSocket
+gnulib_modules="$gnulib_modules ioctl"
+# -> Meson
+gnulib_modules="$gnulib_modules largefile"
+# -> GSocket
+gnulib_modules="$gnulib_modules listen"
+# -> custom configure check
+gnulib_modules="$gnulib_modules localeconv"
+# -> Meson
+gnulib_modules="$gnulib_modules manywarnings"
+# -> painful copy gnulib
+gnulib_modules="$gnulib_modules mgetgroups"
+# -> GSocket
+gnulib_modules="$gnulib_modules net_if"
+# -> GSocket
+gnulib_modules="$gnulib_modules netdb"
+# -> GSocket
+gnulib_modules="$gnulib_modules nonblocking"
+# -> Just add -lutil to cli
+gnulib_modules="$gnulib_modules openpty"
+# -> GSocket
+gnulib_modules="$gnulib_modules passfd"
+# -> open code / copy gnulib code
+gnulib_modules="$gnulib_modules physmem"
+# -> open code / conditional comp
+gnulib_modules="$gnulib_modules pipe-posix"
+# -> open code / conditional comp
+gnulib_modules="$gnulib_modules pipe2"
+# -> GMainLoop
+gnulib_modules="$gnulib_modules poll"
+# -> Meson
+gnulib_modules="$gnulib_modules posix-shell"
+# -> open code conditional logic
+gnulib_modules="$gnulib_modules pthread_sigmask"
+# -> GSocket
+gnulib_modules="$gnulib_modules recv"
+# -> GSocket
+gnulib_modules="$gnulib_modules send"
+# -> GSocket
+gnulib_modules="$gnulib_modules setsockopt"
+# -> open code conditional logic
+gnulib_modules="$gnulib_modules sigaction"
+# -> open code conditional logic
+gnulib_modules="$gnulib_modules sigpipe"
+# -> GSocket
+gnulib_modules="$gnulib_modules socket"
+# -> open code conditional or use GIO GFileInfo
+gnulib_modules="$gnulib_modules stat-time"
+# -> remove use or open-code it. possibly add to glib
+gnulib_modules="$gnulib_modules strchrnul"
+# -> g_strsplit
+gnulib_modules="$gnulib_modules strtok_r"
+# -> remove sys/stat.h include from any win32 code paths
+gnulib_modules="$gnulib_modules sys_stat"
+# -> remove sys/wait.h include from any win32 code paths
+gnulib_modules="$gnulib_modules sys_wait"
+# -> remove from any win32 code paths
+gnulib_modules="$gnulib_modules termios"
+# -> GDateTime ?
+gnulib_modules="$gnulib_modules time_r"
+# -> obsolete - exists on Linux, MacOS >= ?? & FreeBSD >= 6
+gnulib_modules="$gnulib_modules ttyname_r"
+# -> g_get_os_info in GLib 2.64 but can't use that yet
+gnulib_modules="$gnulib_modules uname"
+# -> G_STATIC_ASSERT
+gnulib_modules="$gnulib_modules verify"
+# -> remove from Win32 code paths
+gnulib_modules="$gnulib_modules waitpid"
+# -> Meson
+gnulib_modules="$gnulib_modules warnings"
+# -> open code impl
+gnulib_modules="$gnulib_modules wcwidth"
 
 SKIP_PO=true
 
-- 
2.24.1

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

[libvirt] [PATCH 21/23] src: replace strptime()/timegm()/mktime() with GDateTime APIs set

[Daniel P . Berrangé]

All places where we use strptime/timegm()/mktime() are handling
conversion of dates in a format compatible with ISO 8601, so we
can use the GDateTime APIs to simplify code.

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/domain_conf.c | 32 +--
 src/esx/esx_vi_types.c | 71 +-
 src/vz/vz_sdk.c| 10 +++---
 3 files changed, 20 insertions(+), 93 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index afa072e17d..ee33b7caf0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13673,33 +13673,17 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node,
 
 validTo = virXMLPropString(node, "passwdValidTo");
 if (validTo) {
-char *tmp;
-struct tm tm;
-memset(, 0, sizeof(tm));
-/* Expect: -MM-DDTHH:MM:SS (%d-%d-%dT%d:%d:%d)  eg 
2010-11-28T14:29:01 */
-if (/* year */
-virStrToLong_i(validTo, , 10, _year) < 0 || *tmp != '-' 
||
-/* month */
-virStrToLong_i(tmp+1, , 10, _mon) < 0 || *tmp != '-' ||
-/* day */
-virStrToLong_i(tmp+1, , 10, _mday) < 0 || *tmp != 'T' ||
-/* hour */
-virStrToLong_i(tmp+1, , 10, _hour) < 0 || *tmp != ':' ||
-/* minute */
-virStrToLong_i(tmp+1, , 10, _min) < 0 || *tmp != ':' ||
-/* second */
-virStrToLong_i(tmp+1, , 10, _sec) < 0 || *tmp != '\0') {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot parse password validity time '%s', expect 
-MM-DDTHH:MM:SS"),
-   validTo);
-VIR_FREE(def->passwd);
+g_autoptr(GDateTime) then = NULL;
+g_autoptr(GTimeZone) tz = g_time_zone_new_utc();
+
+then = g_date_time_new_from_iso8601(validTo, tz);
+if (!then) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("password validity time '%s' values out of 
range"), validTo);
 return -1;
 }
 
-tm.tm_year -= 1900; /* Human epoch starts at 0 BC, not 1900BC */
-tm.tm_mon--; /* Humans start months at 1, computers at 0 */
-
-def->validTo = timegm();
+def->validTo = (int)g_date_time_to_unix(then);
 def->expires = true;
 }
 
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 1deb5026b7..434313dfa4 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -1473,27 +1473,14 @@ int
 esxVI_DateTime_ConvertToCalendarTime(esxVI_DateTime *dateTime,
  long long *secondsSinceEpoch)
 {
-char value[64] = "";
-char *tmp;
-struct tm tm;
-int milliseconds;
-char sign;
-int tz_hours;
-int tz_minutes;
-int tz_offset = 0;
+g_autoptr(GDateTime) then = NULL;
+g_autoptr(GTimeZone) tz = NULL;
 
 if (!dateTime || !secondsSinceEpoch) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
 return -1;
 }
 
-if (virStrcpyStatic(value, dateTime->value) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("xsd:dateTime value '%s' too long for destination"),
-   dateTime->value);
-return -1;
-}
-
 /*
  * expected format: [-]CCYY-MM-DDTHH:MM:SS[.ss][((+|-)HH:MM|Z)]
  * typical example: 2010-04-05T12:13:55.316789+02:00
@@ -1502,66 +1489,22 @@ esxVI_DateTime_ConvertToCalendarTime(esxVI_DateTime 
*dateTime,
  *
  * map negative years to 0, since the base for time_t is the year 1970.
  */
-if (*value == '-') {
+if (*(dateTime->value) == '-') {
 *secondsSinceEpoch = 0;
 return 0;
 }
 
-tmp = strptime(value, "%Y-%m-%dT%H:%M:%S", );
+tz = g_time_zone_new_utc();
+then = g_date_time_new_from_iso8601(dateTime->value, tz);
 
-if (!tmp) {
+if (!then) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("xsd:dateTime value '%s' has unexpected format"),
dateTime->value);
 return -1;
 }
 
-if (*tmp != '\0') {
-/* skip .ss part if present */
-if (*tmp == '.' &&
-virStrToLong_i(tmp + 1, , 10, ) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("xsd:dateTime value '%s' has unexpected format"),
-   dateTime->value);
-return -1;
-}
-
-/* parse timezone offset if present. if missing assume UTC */
-if (*tmp == '+' || *tmp == '-') {
-sign = *tmp;
-
-if (virStrToLong_i(tmp + 1, , 10, _hours) < 0 ||
-*tmp != ':' ||
-virStrToLong_i(tmp + 1, NULL, 10, _minutes) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("xsd:dateTime value '%s' has unexpected 
format"),
-   dateTime->value);
-

[libvirt] [PATCH 20/23] src: replace WSAStartup with g_networking_init()

[Daniel P . Berrangé]

g_networking_init() does the same as our custom code.

Signed-off-by: Daniel P. Berrangé 
---
 m4/virt-glib.m4 |  2 +-
 src/libvirt.c   | 25 ++---
 2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/m4/virt-glib.m4 b/m4/virt-glib.m4
index eb2c77b25b..03e51711c0 100644
--- a/m4/virt-glib.m4
+++ b/m4/virt-glib.m4
@@ -24,7 +24,7 @@ AC_DEFUN([LIBVIRT_ARG_GLIB], [
 AC_DEFUN([LIBVIRT_CHECK_GLIB],[
   GLIB_REQUIRED=2.48.0
 
-  LIBVIRT_CHECK_PKG([GLIB], [glib-2.0 gobject-2.0], [$GLIB_REQUIRED])
+  LIBVIRT_CHECK_PKG([GLIB], [glib-2.0 gobject-2.0 gio-2.0], [$GLIB_REQUIRED])
 
   if test "$with_glib" = "no" ; then
 AC_MSG_ERROR([glib-2.0, gobject-2.0 >= $GLIB_REQUIRED are required for 
libvirt])
diff --git a/src/libvirt.c b/src/libvirt.c
index c741ebe311..f1ffc97261 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -26,15 +26,12 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include "getpass.h"
 
-#ifdef HAVE_WINSOCK2_H
-# include 
-#endif
-
 #ifdef WITH_CURL
 # include 
 #endif
@@ -211,21 +208,6 @@ static virConnectAuth virConnectAuthDefault = {
  */
 virConnectAuthPtr virConnectAuthPtrDefault = 
 
-#if HAVE_WINSOCK2_H
-static int
-virWinsockInit(void)
-{
-WORD winsock_version, err;
-WSADATA winsock_data;
-
-/* http://msdn2.microsoft.com/en-us/library/ms742213.aspx */
-winsock_version = MAKEWORD(2, 2);
-err = WSAStartup(winsock_version, _data);
-return err == 0 ? 0 : -1;
-}
-#endif
-
-
 static bool virGlobalError;
 static virOnceControl virGlobalOnce = VIR_ONCE_CONTROL_INITIALIZER;
 
@@ -261,10 +243,7 @@ virGlobalInit(void)
 
 VIR_DEBUG("register drivers");
 
-#if HAVE_WINSOCK2_H
-if (virWinsockInit() == -1)
-goto error;
-#endif
+g_networking_init();
 
 #ifdef HAVE_LIBINTL_H
 if (!bindtextdomain(PACKAGE, LOCALEDIR))
-- 
2.24.1

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

[libvirt] [PATCH 22/23] bootstrap: remove now unused gnulib modules

[Daniel P . Berrangé]

* canonicalize-lgpl: replaced by realpath()/g_canonicalize_filename()
* clock-gettime: replaced by g_get_(real|monotonic)_time
* dirname-lgpl: replaced by g_path_get_dirname()
* fclose: we aren't affected by any portability problems it fixes
* fdatasync: every platform we call fdatasync on has it present
* fsync: replaced by g_fsync()
* fnmatch: replaced by g_pattern_match()
* getcwd-lgpl: replaced by g_get_current_dir()
* gethostname: replaced by g_get_hostname()
* gettimeofday: replaced by g_get_(real|monotonic)_time
* setenv: replaced by g_setenv()
* strptime: replaced by GDateTime
* timegm: replaced by GDateTime
* unsetenv: replaced by g_unsetenv()

Signed-off-by: Daniel P. Berrangé 
---
 bootstrap.conf | 14 --
 1 file changed, 14 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 0a8b591418..3d9243fa01 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -20,27 +20,17 @@
 gnulib_modules='
 accept
 bind
-canonicalize-lgpl
 chown
-clock-time
 close
 connect
 configmake
-dirname-lgpl
 environ
-fclose
 fcntl
 fcntl-h
-fdatasync
-fnmatch
-fsync
 getaddrinfo
-getcwd-lgpl
-gethostname
 getpass
 getpeername
 getsockname
-gettimeofday
 intprops
 ioctl
 largefile
@@ -61,23 +51,19 @@ posix-shell
 pthread_sigmask
 recv
 send
-setenv
 setsockopt
 sigaction
 sigpipe
 socket
 stat-time
 strchrnul
-strptime
 strtok_r
 sys_stat
 sys_wait
 termios
 time_r
-timegm
 ttyname_r
 uname
-unsetenv
 verify
 waitpid
 warnings
-- 
2.24.1

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

[libvirt] [PATCH 17/23] src: replace getcwd() with g_get_current_dir()

[Daniel P . Berrangé]

commandhelper.c is not converted since this is a standalone
program only run on UNIX, so can rely on getcwd().

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virfile.c |  5 +
 tools/vsh.c| 16 +++-
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index d5e4d0a289..9787086f3d 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3187,10 +3187,7 @@ virFileAbsPath(const char *path, char **abspath)
 if (path[0] == '/') {
 *abspath = g_strdup(path);
 } else {
-g_autofree char *buf = getcwd(NULL, 0);
-
-if (buf == NULL)
-return -1;
+g_autofree char *buf = g_get_current_dir();
 
 *abspath = g_strdup_printf("%s/%s", buf, path);
 }
diff --git a/tools/vsh.c b/tools/vsh.c
index a2f33e01aa..a36b6bfe23 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3296,21 +3296,11 @@ const vshCmdInfo info_pwd[] = {
 bool
 cmdPwd(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
 {
-char *cwd;
-bool ret = true;
-char ebuf[1024];
+g_autofree char *cwd = g_get_current_dir();
 
-cwd = getcwd(NULL, 0);
-if (!cwd) {
-vshError(ctl, _("pwd: cannot get current directory: %s"),
- virStrerror(errno, ebuf, sizeof(ebuf)));
-ret = false;
-} else {
-vshPrint(ctl, _("%s\n"), cwd);
-VIR_FREE(cwd);
-}
+vshPrint(ctl, _("%s\n"), cwd);
 
-return ret;
+return true;
 }
 
 const vshCmdInfo info_quit[] = {
-- 
2.24.1

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

[libvirt] [PATCH 18/23] util: use realpath/g_canonicalize_filename

[Daniel P . Berrangé]

The canonicalize_file_name(path) is equivalent to calling
realpath(path, NULL). Passing NULL for the second arg of
realpath is not standardized behaviour, however, Linux,
FreeBSD > 6.4 and macOS > 10.5 all support this critical
extension.

This leaves Windows which doesn't provide realpath at all.
The g_canonicalize_filename() function doesn't expand
symlinks, so is not strictly equivalent to realpath()
but is close enough for our Windows portability needs
right now.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virfile.c  |  9 -
 tests/virfilemock.c | 12 +++-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 9787086f3d..4fc872ef1d 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3256,7 +3256,14 @@ virFileSanitizePath(const char *path)
 char *
 virFileCanonicalizePath(const char *path)
 {
-return canonicalize_file_name(path); /* exempt from syntax-check */
+#ifdef WIN32
+/* Does not resolve symlinks, only expands . & .. & repeated /.
+ * It will never fail, so sanitize errno to indicate success */
+errno = 0;
+return g_canonicalize_filename(path, NULL);
+#else
+return realpath(path, NULL); /* exempt from syntax-check */
+#endif
 }
 
 /**
diff --git a/tests/virfilemock.c b/tests/virfilemock.c
index 65685c6d26..802ee9b5f9 100644
--- a/tests/virfilemock.c
+++ b/tests/virfilemock.c
@@ -24,6 +24,7 @@
 #if HAVE_LINUX_MAGIC_H
 # include 
 #endif
+#include 
 
 #include "virmock.h"
 #include "virstring.h"
@@ -33,7 +34,7 @@
 
 static FILE *(*real_setmntent)(const char *filename, const char *type);
 static int (*real_statfs)(const char *path, struct statfs *buf);
-static char *(*real_canonicalize_file_name)(const char *path);
+static char *(*real_realpath)(const char *path, char *resolved);
 
 
 static void
@@ -44,7 +45,7 @@ init_syms(void)
 
 VIR_MOCK_REAL_INIT(setmntent);
 VIR_MOCK_REAL_INIT(statfs);
-VIR_MOCK_REAL_INIT(canonicalize_file_name);
+VIR_MOCK_REAL_INIT(realpath);
 }
 
 
@@ -116,7 +117,7 @@ statfs_mock(const char *mtab,
 /* We don't need to do this in callers because real statfs(2)
  * does that for us. However, in mocked implementation we
  * need to do this. */
-if (!(canonPath = canonicalize_file_name(path)))
+if (!(canonPath = realpath(path, NULL)))
 return -1;
 
 while (getmntent_r(f, , mntbuf, sizeof(mntbuf))) {
@@ -178,7 +179,7 @@ statfs(const char *path, struct statfs *buf)
 
 
 char *
-canonicalize_file_name(const char *path)
+realpath(const char *path, char *resolved)
 {
 
 init_syms();
@@ -187,6 +188,7 @@ canonicalize_file_name(const char *path)
 const char *p;
 char *ret;
 
+assert(resolved == NULL);
 if ((p = STRSKIP(path, "/some/symlink")))
 ret = g_strdup_printf("/gluster%s", p);
 else
@@ -195,5 +197,5 @@ canonicalize_file_name(const char *path)
 return ret;
 }
 
-return real_canonicalize_file_name(path);
+return real_realpath(path, resolved);
 }
-- 
2.24.1

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

[libvirt] [PATCH 16/23] src: remove unused imports of dirname.h

[Daniel P . Berrangé]

A few places were importing dirname.h without actually using it.

Signed-off-by: Daniel P. Berrangé 
---
 build-aux/syntax-check.mk| 2 +-
 src/qemu/qemu_command.c  | 1 -
 src/storage/storage_driver.c | 1 -
 tests/virstoragetest.c   | 1 -
 4 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 8de82e9872..7e7c59c3df 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -611,7 +611,7 @@ sc_forbid_manual_xml_indent:
 # dirname and basename from  are not required to be thread-safe
 sc_prohibit_libgen:
@prohibit='( (base|dir)name *\(|include .libgen\.h)' \
-   halt='use functions from gnulib "dirname.h", not ' \
+   halt='use functions from GLib, not ' \
  $(_sc_search_regexp)
 
 # raw xmlGetProp requires some nasty casts
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a8137b3a32..11d3c29297 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -31,7 +31,6 @@
 #include "qemu_slirp.h"
 #include "qemu_block.h"
 #include "cpu/cpu.h"
-#include "dirname.h"
 #include "viralloc.h"
 #include "virlog.h"
 #include "virarch.h"
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 72ba252543..6bbf52f729 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -48,7 +48,6 @@
 #include "virsecret.h"
 #include "virstring.h"
 #include "viraccessapicheck.h"
-//#include "dirname.h"
 #include "storage_util.h"
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index b9d4a45cdd..0e274ad1b7 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -26,7 +26,6 @@
 #include "virlog.h"
 #include "virstoragefile.h"
 #include "virstring.h"
-#include "dirname.h"
 
 #include "storage/storage_driver.h"
 
-- 
2.24.1

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

[libvirt] [PATCH 19/23] util: replace gethostname() with g_get_hostname()

[Daniel P . Berrangé]

Note the glib function returns a const string because it
caches the hostname using a one time thread initializer
function.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virlog.c  | 17 -
 src/util/virutil.c | 12 +++-
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index aa98024e1c..e46ae69471 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -60,7 +60,6 @@
 VIR_LOG_INIT("util.log");
 
 static GRegex *virLogRegex;
-static char virLogHostname[HOST_NAME_MAX+1];
 
 
 #define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}"
@@ -253,8 +252,6 @@ virLogPriorityString(virLogPriority lvl)
 static int
 virLogOnceInit(void)
 {
-int r;
-
 if (virMutexInit() < 0)
 return -1;
 
@@ -263,19 +260,13 @@ virLogOnceInit(void)
 
 virLogRegex = g_regex_new(VIR_LOG_REGEX, G_REGEX_OPTIMIZE, 0, NULL);
 
-/* We get and remember the hostname early, because at later time
+/* GLib caches the hostname using a one time thread initializer.
+ * We want to prime this cache early though, because at later time
  * it might not be possible to load NSS modules via getaddrinfo()
  * (e.g. at container startup the host filesystem will not be
  * accessible anymore.
- * Must not use virGetHostname though as that causes re-entrancy
- * problems if it triggers logging codepaths
  */
-r = gethostname(virLogHostname, sizeof(virLogHostname));
-if (r == -1) {
-ignore_value(virStrcpyStatic(virLogHostname, "(unknown)"));
-} else {
-NUL_TERMINATE(virLogHostname);
-}
+(void)g_get_host_name();
 
 virLogUnlock();
 return 0;
@@ -471,7 +462,7 @@ virLogHostnameString(char **rawmsg,
 {
 char *hoststr;
 
-hoststr = g_strdup_printf("hostname: %s", virLogHostname);
+hoststr = g_strdup_printf("hostname: %s", g_get_host_name());
 
 virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr);
 *rawmsg = hoststr;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 9620ff204d..a0fd7618ee 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -503,17 +503,11 @@ static char *
 virGetHostnameImpl(bool quiet)
 {
 int r;
-char hostname[HOST_NAME_MAX+1], *result = NULL;
+const char *hostname;
+char *result = NULL;
 struct addrinfo hints, *info;
 
-r = gethostname(hostname, sizeof(hostname));
-if (r == -1) {
-if (!quiet)
-virReportSystemError(errno,
- "%s", _("failed to determine host name"));
-return NULL;
-}
-NUL_TERMINATE(hostname);
+hostname = g_get_host_name();
 
 if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) {
 /* in this case, gethostname returned localhost (meaning we can't
-- 
2.24.1

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

[libvirt] [PATCH 04/23] util: add note about event file descriptors on Windows

[Daniel P . Berrangé]

When using GNULIB with Winsock, libvirt will never see the normal HANDLE
objects, instead GNULIB guarantees that libvirt gets a C runtime file
descriptor. The GNULIB poll impl also expects to get C runtime file
descriptors rather than HANDLE objects. Document this behaviour so that
it is clear to applications providing event loop implementations if they
need Windows portability.

Signed-off-by: Daniel P. Berrangé 
---
 build-aux/syntax-check.mk   | 2 +-
 include/libvirt/libvirt-event.h | 4 
 src/util/virevent.c | 7 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 19a11ce348..8de82e9872 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -2238,7 +2238,7 @@ exclude_file_name_regexp--sc_prohibit_strdup = \
   
^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c|tests/commandhelper\.c|tools/nss/libvirt_nss_(leases|macs)\.c$$)
 
 exclude_file_name_regexp--sc_prohibit_close = \
-  
(\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$)
+  
(\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$)
 
 exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \
   
(^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
diff --git a/include/libvirt/libvirt-event.h b/include/libvirt/libvirt-event.h
index 734dbdcbc1..facdc3a3ec 100644
--- a/include/libvirt/libvirt-event.h
+++ b/include/libvirt/libvirt-event.h
@@ -67,6 +67,10 @@ typedef void (*virEventHandleCallback)(int watch, int fd, 
int events, void *opaq
  * listen for specific events. The same file handle can be registered
  * multiple times provided the requested event sets are non-overlapping
  *
+ * @fd will always be a C runtime file descriptor. On Windows
+ * the _get_osfhandle() method can be used if a HANDLE is required
+ * instead.
+ *
  * If the opaque user data requires free'ing when the handle
  * is unregistered, then a 2nd callback can be supplied for
  * this purpose. This callback needs to be invoked from a clean stack.
diff --git a/src/util/virevent.c b/src/util/virevent.c
index f6c797724e..fd5d8f5bf1 100644
--- a/src/util/virevent.c
+++ b/src/util/virevent.c
@@ -60,6 +60,13 @@ static virEventRemoveTimeoutFunc removeTimeoutImpl;
  * requires that an event loop has previously been registered with
  * virEventRegisterImpl() or virEventRegisterDefaultImpl().
  *
+ * @fd must always always be a C runtime file descriptor. On Windows
+ * if the caller only has a HANDLE, the _open_osfhandle() method can
+ * be used to open an associated C runtime file descriptor for use
+ * with this API. After opening a runtime file descriptor, CloseHandle()
+ * must not be used, instead close() will close the runtime file
+ * descriptor and its original associated HANDLE.
+ *
  * Returns -1 if the file handle cannot be registered, otherwise a handle
  * watch number to be used for updating and unregistering for events.
  */
-- 
2.24.1

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

[libvirt] [PATCH 09/23] util: introduce virFileDataSync

[Daniel P . Berrangé]

A wrapper that calls g_fsync on Win32/macOS and fdatasync
elsewhere. g_fsync is a stronger flush than we need but it
satisfies the caller's requirements & matches the approach
gnulib takes.

Signed-off-by: Daniel P. Berrangé 
---
 src/libvirt_private.syms   |  1 +
 src/storage/storage_util.c |  4 ++--
 src/util/iohelper.c|  2 +-
 src/util/virfile.c | 11 +++
 src/util/virfile.h |  2 ++
 5 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d3184dbf5c..951ba7f0ca 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1976,6 +1976,7 @@ virFileChownFiles;
 virFileClose;
 virFileComparePaths;
 virFileCopyACLs;
+virFileDataSync;
 virFileDeleteTree;
 virFileDirectFdFlag;
 virFileExists;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c1a6b44f4b..45ffd2206e 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -216,7 +216,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 } while ((amtleft -= interval) > 0);
 }
 
-if (fdatasync(fd) < 0) {
+if (virFileDataSync(fd) < 0) {
 ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
@@ -2539,7 +2539,7 @@ storageBackendWipeLocal(const char *path,
 remaining -= written;
 }
 
-if (fdatasync(fd) < 0) {
+if (virFileDataSync(fd) < 0) {
 virReportSystemError(errno,
  _("cannot sync data to volume with path '%s'"),
  path);
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index a102a95abb..d864bbeaed 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -154,7 +154,7 @@ runIO(const char *path, int fd, int oflags)
 }
 
 /* Ensure all data is written */
-if (fdatasync(fdout) < 0) {
+if (virFileDataSync(fdout) < 0) {
 if (errno != EINVAL && errno != EROFS) {
 /* fdatasync() may fail on some special FDs, e.g. pipes */
 virReportSystemError(errno, _("unable to fsync %s"), fdoutname);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 1784895575..c79fe86403 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4432,3 +4432,14 @@ virFileGetXAttr(const char *path,
 
 return ret;
 }
+
+
+int
+virFileDataSync(int fd)
+{
+#if defined(__APPLE__) || defined(WIN32)
+return g_fsync(fd);
+#else
+return fdatasync(fd);
+#endif
+}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 264b12c03d..c73f5252e4 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -372,3 +372,5 @@ int virFileSetXAttr(const char *path,
 int virFileRemoveXAttr(const char *path,
const char *name)
 G_GNUC_NO_INLINE;
+
+int virFileDataSync(int fd);
-- 
2.24.1

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

[libvirt] [PATCH 01/23] build: set min version for CLang to 3.4 / XCode CLang to 5.1

[Daniel P . Berrangé]

We have a strong check for GCC >= 4.8, but don't validate any
version number for CLang historically.

This defines the min CLang to be 3.4 which is what is available
for RHEL-7. macOS uses a different versioning scheme for CLang,
based off XCode versions. There is a mapping recorded at

  https://en.wikipedia.org/wiki/Xcode#Toolchain_versions

Here we see upstream CLang 3.4 corresponds to XCode 5.1

XCode 5.1 is available for macOS 10.8.4 or later which
trivially satisfies our platform support matrix requirements.

All these versions match what QEMU declares for its min GCC
and CLang checks.

Signed-off-by: Daniel P. Berrangé 
---
 config-post.h | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/config-post.h b/config-post.h
index 7c07e05340..415cc8cc72 100644
--- a/config-post.h
+++ b/config-post.h
@@ -32,6 +32,20 @@
 ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
 #endif
 
-#if !(__GNUC_PREREQ(4, 8) || defined(__clang__))
-# error "Libvirt requires GCC >= 4.8, or CLang"
+#if defined(__clang_major__) && defined(__clang_minor__)
+# ifdef __apple_build_version__
+#  if __clang_major__ < 5 || (__clang_major__ == 5 && __clang_minor__ < 1)
+#   error You need at least XCode Clang v5.1 to compile QEMU
+#  endif
+# else
+#  if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 4)
+#   error You need at least Clang v3.4 to compile QEMU
+#  endif
+# endif
+#elif defined(__GNUC__) && defined(__GNUC_MINOR__)
+# if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
+#  error You need at least GCC v4.8 to compile QEMU
+# endif
+#else
+# error You either need at least GCC 4.8 or Clang 3.4 or XCode Clang 5.1 to 
compile libvirt
 #endif
-- 
2.24.1

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

[libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining

[Daniel P . Berrangé]

In the last days before the xmas break I took some time to
eliminate 14 more gnulib modules, bringing us down to just
50 left to go. They're getting harder to eliminate as we go
on, but to give some hints, I've annotated every module in
bootstrap.conf with a suggested replacement strategy.

Daniel P. Berrangé (23):
  build: set min version for CLang to 3.4 / XCode CLang to 5.1
  docs: expand macOS platform support coverage
  travis: add macOS Xcode 11.3 testing
  util: add note about event file descriptors on Windows
  src: always pull in glib/gstdio.h header
  src: switch to use g_setenv/g_unsetenv
  util: add compat wrapper for g_fsync
  src: use g_fsync for portability
  util: introduce virFileDataSync
  src: use g_lstat() instead of lstat()
  src: switch from fnmatch to g_pattern_match_simple
  src: replace clock_gettime()/gettimeofday() with g_get_real_time()
  src: replace last_component() with g_path_get_basename()
  util: replace IS_ABSOLUTE_FILE_NAME with g_path_is_absolute
  src: replace mdir_name() with g_path_get_dirname()
  src: remove unused imports of dirname.h
  src: replace getcwd() with g_get_current_dir()
  util: use realpath/g_canonicalize_filename
  util: replace gethostname() with g_get_hostname()
  src: replace WSAStartup with g_networking_init()
  src: replace strptime()/timegm()/mktime() with GDateTime APIs set
  bootstrap: remove now unused gnulib modules
  bootstrap: annotate with info about desired replacement

 .travis.yml |  15 ++
 bootstrap.conf  | 173 +++-
 build-aux/syntax-check.mk   |   4 +-
 config-post.h   |  18 ++-
 docs/platforms.html.in  |  13 +-
 include/libvirt/libvirt-event.h |   4 +
 m4/virt-glib.m4 |   2 +-
 src/conf/domain_conf.c  |  32 ++---
 src/conf/moment_conf.c  |   7 +-
 src/conf/node_device_conf.c |   6 +-
 src/esx/esx_vi_types.c  |  71 +-
 src/internal.h  |   4 +-
 src/libvirt.c   |  25 +---
 src/libvirt_private.syms|   2 +
 src/libxl/libxl_driver.c|  21 +--
 src/locking/lock_driver_sanlock.c   |   6 +-
 src/node_device/node_device_udev.c  |  12 +-
 src/nwfilter/nwfilter_dhcpsnoop.c   |   2 +-
 src/qemu/qemu_backup.c  |   5 +-
 src/qemu/qemu_command.c |   1 -
 src/qemu/qemu_domain.c  |  14 +-
 src/qemu/qemu_driver.c  |   7 +-
 src/qemu/qemu_firmware.c|   4 +-
 src/qemu/qemu_tpm.c |  33 +
 src/remote/libvirtd.conf.in |   8 +-
 src/rpc/virnetsaslcontext.c |  11 +-
 src/rpc/virnetsocket.c  |   5 +-
 src/rpc/virnettlscontext.c  |  10 +-
 src/security/security_dac.c |   2 +-
 src/security/virt-aa-helper.c   |   8 +-
 src/storage/storage_backend_disk.c  |   9 +-
 src/storage/storage_driver.c|   1 -
 src/storage/storage_util.c  |  11 +-
 src/test/test_driver.c  |  51 ++-
 src/util/glibcompat.c   |  13 ++
 src/util/glibcompat.h   |   6 +
 src/util/iohelper.c |   2 +-
 src/util/vircgroupv1.c  |   4 +-
 src/util/virevent.c |   7 +
 src/util/virfile.c  |  49 ---
 src/util/virfile.h  |   2 +
 src/util/virlog.c   |  22 +--
 src/util/virmdev.c  |  12 +-
 src/util/virnetdev.c|   5 +-
 src/util/virpci.c   |  17 ++-
 src/util/virstoragefile.c   |  13 +-
 src/util/virsystemd.c   |   4 +-
 src/util/virtime.c  |  22 +--
 src/util/virutil.c  |  12 +-
 src/vbox/vbox_XPCOMCGlue.c  |   4 +-
 src/vmware/vmware_conf.c|   7 +-
 src/vz/vz_sdk.c |  10 +-
 tests/eventtest.c   |  18 +--
 tests/libxlxml2domconfigtest.c  |   4 +-
 tests/lxcxml2xmltest.c  |   2 +-
 tests/qemudomaincheckpointxml2xmltest.c |   2 +-
 tests/qemudomainsnapshotxml2xmltest.c   |   2 +-
 tests/qemufirmwaretest.c|   2 +-
 tests/qemuhotplugtest.c |   2 +-
 tests/qemumemlocktest.c |   2 +-
 tests/qemusecuritytest.c|   4 +-
 tests/qemuvhostusertest.c   |   2 +-
 tests/qemuxml2argvtest.c|  24 ++--
 tests/qemuxml2xmltest.c |   4 +-
 tests/securityselinuxhelper.c   |   4 +-
 tests/testutils.c   |  15 +-
 tests/testutils.h   |   6 +-
 tests/testutilsqemu.c   |   4 +-
 tests/vircgrouptest.c   |  16 +--
 tests/virconfdata/libvirtd.conf

[libvirt] [PATCH 06/23] src: switch to use g_setenv/g_unsetenv

[Daniel P . Berrangé]

Eliminate direct use of normal setenv/unsetenv calls in
favour of GLib's wrapper. This eliminates two gnulib
modules

Signed-off-by: Daniel P. Berrangé 
---
 src/security/virt-aa-helper.c   |  4 +--
 src/util/virsystemd.c   |  4 +--
 src/vbox/vbox_XPCOMCGlue.c  |  4 +--
 tests/libxlxml2domconfigtest.c  |  4 +--
 tests/lxcxml2xmltest.c  |  2 +-
 tests/qemudomaincheckpointxml2xmltest.c |  2 +-
 tests/qemudomainsnapshotxml2xmltest.c   |  2 +-
 tests/qemufirmwaretest.c|  2 +-
 tests/qemuhotplugtest.c |  2 +-
 tests/qemumemlocktest.c |  2 +-
 tests/qemusecuritytest.c|  4 +--
 tests/qemuvhostusertest.c   |  2 +-
 tests/qemuxml2argvtest.c| 24 ++---
 tests/qemuxml2xmltest.c |  4 +--
 tests/securityselinuxhelper.c   |  4 +--
 tests/testutils.c   |  8 ++---
 tests/testutils.h   |  4 +--
 tests/testutilsqemu.c   |  4 +--
 tests/vircgrouptest.c   | 16 -
 tests/virfiletest.c |  4 +--
 tests/virhostdevtest.c  |  2 +-
 tests/virnettlscontexttest.c|  2 +-
 tests/virnettlssessiontest.c|  2 +-
 tests/virpcitest.c  |  2 +-
 tests/virportallocatortest.c|  2 +-
 tests/virsystemdtest.c  | 46 -
 tests/virtimetest.c |  4 +--
 tools/virt-login-shell-helper.c | 12 +++
 28 files changed, 87 insertions(+), 87 deletions(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index f623ff965f..feb03b0aa9 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1447,11 +1447,11 @@ main(int argc, char **argv)
 
 /* clear the environment */
 environ = NULL;
-if (setenv("PATH", "/sbin:/usr/sbin", 1) != 0)
+if (g_setenv("PATH", "/sbin:/usr/sbin", 1) == FALSE)
 vah_error(ctl, 1, _("could not set PATH"));
 
 /* ensure the traditional IFS setting */
-if (setenv("IFS", " \t\n", 1) != 0)
+if (g_setenv("IFS", " \t\n", 1) == FALSE)
 vah_error(ctl, 1, _("could not set IFS"));
 
 if (!(progname = strrchr(argv[0], '/')))
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 2acb54d41e..96d43e5440 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -854,8 +854,8 @@ virSystemdGetListenFDs(void)
 return 0;
 }
 
-unsetenv("LISTEN_PID");
-unsetenv("LISTEN_FDS");
+g_unsetenv("LISTEN_PID");
+g_unsetenv("LISTEN_FDS");
 
 VIR_DEBUG("Got %u file descriptors", nfds);
 
diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c
index f48a78a923..4151d20909 100644
--- a/src/vbox/vbox_XPCOMCGlue.c
+++ b/src/vbox/vbox_XPCOMCGlue.c
@@ -104,9 +104,9 @@ tryLoadOne(const char *dir, bool setAppHome, bool 
ignoreMissing,
  */
 if (setAppHome) {
 if (dir != NULL) {
-setenv("VBOX_APP_HOME", dir, 1 /* always override */);
+g_setenv("VBOX_APP_HOME", dir, 1 /* always override */);
 } else {
-unsetenv("VBOX_APP_HOME");
+g_unsetenv("VBOX_APP_HOME");
 }
 }
 
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index 35f0ce8f7d..f3b8c8bffd 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -160,8 +160,8 @@ mymain(void)
  * results. In order to detect things that just work by a blind
  * chance, we need to set an virtual timezone that no libvirt
  * developer resides in. */
-if (setenv("TZ", "VIR00:30", 1) < 0) {
-perror("setenv");
+if (g_setenv("TZ", "VIR00:30", 1) == FALSE) {
+perror("g_setenv");
 return EXIT_FAILURE;
 }
 
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index 7b05f7d016..f7457e719a 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -75,7 +75,7 @@ mymain(void)
 /* Unset or set all envvars here that are copied in lxcdBuildCommandLine
  * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
  * values for these envvars */
-setenv("PATH", "/bin", 1);
+g_setenv("PATH", "/bin", 1);
 
 DO_TEST("systemd");
 DO_TEST("hostdev");
diff --git a/tests/qemudomaincheckpointxml2xmltest.c 
b/tests/qemudomaincheckpointxml2xmltest.c
index 4d6904a592..b27c0e362a 100644
--- a/tests/qemudomaincheckpointxml2xmltest.c
+++ b/tests/qemudomaincheckpointxml2xmltest.c
@@ -175,7 +175,7 @@ mymain(void)
 /* Unset or set all envvars here that are copied in qemudBuildCommandLine
  * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
  * values for these envvars */
-setenv("PATH", "/bin", 1);
+g_setenv("PATH", "/bin", 1);
 
 /* Test a normal user redefine */
 DO_TEST_OUT("redefine", 0);
diff --git 

[libvirt] [PATCH 13/23] src: replace last_component() with g_path_get_basename()

[Daniel P . Berrangé]

The last_component() method is a GNULIB custom function
that returns a pointer to the base name in the path.
This is similar to g_path_get_basename() but without the
malloc. The extra malloc is no trouble for libvirt's
needs so we can use g_path_get_basename().

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/node_device_conf.c|  2 +-
 src/node_device/node_device_udev.c | 12 ++--
 src/rpc/virnetsocket.c |  5 ++---
 src/storage/storage_backend_disk.c |  9 -
 src/storage/storage_util.c |  3 +--
 src/util/virmdev.c | 12 +++-
 src/util/virnetdev.c   |  5 ++---
 src/util/virpci.c  | 17 -
 tests/testutils.c  |  7 +++
 tests/testutils.h  |  2 --
 tests/virpcimock.c |  3 +--
 11 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index a02c8fe306..9b206abadc 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2448,7 +2448,7 @@ virNodeDeviceGetSCSITargetCaps(const char *sysfsPath,
 if (!(dir = mdir_name(sysfsPath)))
 return -1;
 
-rport = g_strdup(last_component(dir));
+rport = g_path_get_basename(dir);
 
 if (!virFCIsCapableRport(rport))
 goto cleanup;
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index eedcd123a3..4b33dc25d8 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 
-#include "dirname.h"
 #include "node_device_conf.h"
 #include "node_device_event.h"
 #include "node_device_driver.h"
@@ -602,10 +601,10 @@ udevProcessSCSIHost(struct udev_device *device 
G_GNUC_UNUSED,
 virNodeDeviceDefPtr def)
 {
 virNodeDevCapSCSIHostPtr scsi_host = >caps->data.scsi_host;
-char *filename = NULL;
+g_autofree char *filename = NULL;
 char *str;
 
-filename = last_component(def->sysfs_path);
+filename = g_path_get_basename(def->sysfs_path);
 
 if (!(str = STRSKIP(filename, "host")) ||
 virStrToLong_ui(str, NULL, 0, _host->host) < 0) {
@@ -711,9 +710,10 @@ udevProcessSCSIDevice(struct udev_device *device 
G_GNUC_UNUSED,
 int ret = -1;
 unsigned int tmp = 0;
 virNodeDevCapSCSIPtr scsi = >caps->data.scsi;
-char *filename = NULL, *p = NULL;
+g_autofree char *filename = NULL;
+char *p = NULL;
 
-filename = last_component(def->sysfs_path);
+filename = g_path_get_basename(def->sysfs_path);
 
 if (virStrToLong_ui(filename, , 10, >host) < 0 || p == NULL ||
 virStrToLong_ui(p + 1, , 10, >bus) < 0 || p == NULL ||
@@ -1038,7 +1038,7 @@ udevProcessMediatedDevice(struct udev_device *dev,
 goto cleanup;
 }
 
-data->type = g_strdup(last_component(canonicalpath));
+data->type = g_path_get_basename(canonicalpath);
 
 uuidstr = udev_device_get_sysname(dev);
 if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(uuidstr)) < 0)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f072afe857..16a527e399 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -55,7 +55,6 @@
 #include "virprobe.h"
 #include "virprocess.h"
 #include "virstring.h"
-#include "dirname.h"
 #include "passfd.h"
 
 #if WITH_SSH2
@@ -668,7 +667,7 @@ int virNetSocketNewConnectUNIX(const char *path,
 remoteAddr.len = sizeof(remoteAddr.data.un);
 
 if (spawnDaemon) {
-const char *binname;
+g_autofree char *binname = NULL;
 
 if (spawnDaemon && !binary) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -677,7 +676,7 @@ int virNetSocketNewConnectUNIX(const char *path,
 goto cleanup;
 }
 
-if (!(binname = last_component(binary)) || binname[0] == '\0') {
+if (!(binname = g_path_get_basename(binary)) || binname[0] == '\0') {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot determine basename for binary '%s'"),
binary);
diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index 45d1257f3d..00e8b1aa13 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -22,7 +22,6 @@
 #include 
 #include 
 
-#include "dirname.h"
 #include "virerror.h"
 #include "virlog.h"
 #include "storage_backend_disk.h"
@@ -777,10 +776,10 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
unsigned int flags)
 {
 char *part_num = NULL;
-char *dev_name;
+g_autofree char *dev_name = NULL;
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 char *src_path = def->source.devices[0].path;
-char *srcname = last_component(src_path);
+g_autofree char *srcname = g_path_get_basename(src_path);
 bool isDevMapperDevice;
 g_autofree char *devpath = NULL;
 

[libvirt] [PATCH 12/23] src: replace clock_gettime()/gettimeofday() with g_get_real_time()

[Daniel P . Berrangé]

g_get_real_time() returns the time since epoch in microseconds.
It uses gettimeofday() internally while libvirt used clock_gettime
because it is declared async signal safe. In practice gettimeofday
is also async signal safe *provided* the timezone parameter is
NULL. This is indeed the case in g_get_real_time().

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/moment_conf.c   |  7 ++
 src/libxl/libxl_driver.c | 21 +
 src/qemu/qemu_backup.c   |  5 ++--
 src/test/test_driver.c   | 51 
 src/util/virtime.c   | 22 +
 tests/eventtest.c| 18 +-
 tools/virsh-domain.c | 10 
 tools/vsh.c  | 11 -
 tools/vsh.h  |  1 -
 9 files changed, 40 insertions(+), 106 deletions(-)

diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c
index d02fcb89e2..fb6f7824cb 100644
--- a/src/conf/moment_conf.c
+++ b/src/conf/moment_conf.c
@@ -73,13 +73,10 @@ virDomainMomentDefDispose(void *obj)
 int
 virDomainMomentDefPostParse(virDomainMomentDefPtr def)
 {
-struct timeval tv;
-
-gettimeofday(, NULL);
+def->creationTime = g_get_real_time() / (1000*1000);
 
 if (!def->name)
-def->name = g_strdup_printf("%lld", (long long)tv.tv_sec);
+def->name = g_strdup_printf("%lld", def->creationTime);
 
-def->creationTime = tv.tv_sec;
 return 0;
 }
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b2f191b2ac..f021ec9c5d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -235,9 +235,9 @@ libxlTimeoutRegisterEventHook(void *priv,
   void *xl_priv)
 {
 libxlOSEventHookInfoPtr info;
-struct timeval now;
-struct timeval res;
-static struct timeval zero;
+gint64 now_us;
+gint64 abs_us;
+gint64 res_ms;
 int timeout;
 
 if (VIR_ALLOC(info) < 0)
@@ -246,15 +246,16 @@ libxlTimeoutRegisterEventHook(void *priv,
 info->ctx = priv;
 info->xl_priv = xl_priv;
 
-gettimeofday(, NULL);
-timersub(_t, , );
-/* Ensure timeout is not overflowed */
-if (timercmp(, , <)) {
+now_us = g_get_real_time();
+abs_us = (abs_t.tv_sec * (1000LL*1000LL)) + abs_t.tv_usec;
+if (now_us >= abs_us) {
 timeout = 0;
-} else if (res.tv_sec > INT_MAX / 1000) {
-timeout = INT_MAX;
 } else {
-timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
+res_ms = (abs_us - now_us) / 1000;
+if (res_ms > INT_MAX)
+timeout = INT_MAX;
+else
+timeout = res_ms;
 }
 info->id = virEventAddTimeout(timeout, libxlTimerCallback,
   info, libxlOSEventHookInfoFree);
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 294d5999a0..a039beaede 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -715,7 +715,6 @@ qemuBackupBegin(virDomainObjPtr vm,
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
 g_autoptr(virDomainBackupDef) def = NULL;
 g_autofree char *suffix = NULL;
-struct timeval tv;
 bool pull = false;
 virDomainMomentObjPtr chk = NULL;
 g_autoptr(virDomainCheckpointDef) chkdef = NULL;
@@ -749,8 +748,8 @@ qemuBackupBegin(virDomainObjPtr vm,
 
 suffix = g_strdup(chkdef->parent.name);
 } else {
-gettimeofday(, NULL);
-suffix = g_strdup_printf("%lld", (long long)tv.tv_sec);
+gint64 now_us = g_get_real_time();
+suffix = g_strdup_printf("%lld", (long long)now_us/(1000*1000));
 }
 
 if (def->type == VIR_DOMAIN_BACKUP_TYPE_PULL)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2e4ecfd0ff..f8e08dcf07 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2052,29 +2052,19 @@ testDomainGetHostname(virDomainPtr domain,
 static int testDomainGetInfo(virDomainPtr domain,
  virDomainInfoPtr info)
 {
-struct timeval tv;
 virDomainObjPtr privdom;
-int ret = -1;
 
 if (!(privdom = testDomObjFromDomain(domain)))
 return -1;
 
-if (gettimeofday(, NULL) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("getting time of day"));
-goto cleanup;
-}
-
 info->state = virDomainObjGetState(privdom, NULL);
 info->memory = privdom->def->mem.cur_balloon;
 info->maxMem = virDomainDefGetMemoryTotal(privdom->def);
 info->nrVirtCpu = virDomainDefGetVcpus(privdom->def);
-info->cpuTime = ((tv.tv_sec * 1000ll * 1000ll  * 1000ll) + (tv.tv_usec * 
1000ll));
-ret = 0;
+info->cpuTime = g_get_real_time() * 1000;
 
- cleanup:
 virDomainObjEndAPI();
-return ret;
+return 0;
 }
 
 static int
@@ -2933,7 +2923,6 @@ static int testDomainGetVcpus(virDomainPtr domain,
 size_t i;
 int hostcpus;
 int ret = -1;
-struct timeval tv;
 unsigned long long statbase;
 virBitmapPtr allcpumap = NULL;
 

[libvirt] [PATCH 03/23] travis: add macOS Xcode 11.3 testing

[Daniel P . Berrangé]

Ideally we would test macOS 10.15 as the newest release, however, that
is not available in Travis yet. We can at least test newer XCode
versions though to get toolchain validation.

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index f129706456..e1e8cb4f80 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -75,6 +75,21 @@ matrix:
 # macOS, but doing 'install' and 'dist' gives us some useful coverage
 - mkdir build && cd build
 - ../autogen.sh --prefix=$(pwd)/install-root && make -j3 && make -j3 
install && make -j3 dist
+- compiler: clang
+  language: c
+  os: osx
+  osx_image: xcode11.3
+  env:
+- 
PATH="/usr/local/opt/gettext/bin:/usr/local/opt/ccache/libexec:/usr/local/opt/rpcgen/bin:$PATH"
+- PKG_CONFIG_PATH="/usr/local/opt/libxml2/lib/pkgconfig"
+  before_script:
+# Hack to blow away py2
+- brew link --overwrite python
+  script:
+# We can't run 'distcheck' or 'syntax-check' because they fail on
+# macOS, but doing 'install' and 'dist' gives us some useful coverage
+- mkdir build && cd build
+- ../autogen.sh --prefix=$(pwd)/install-root && make -j3 && make -j3 
install && make -j3 dist
 
 git:
   submodules: true
-- 
2.24.1

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

[libvirt] [PATCH 15/23] src: replace mdir_name() with g_path_get_dirname()

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/node_device_conf.c   |  4 +---
 src/locking/lock_driver_sanlock.c |  6 +-
 src/qemu/qemu_driver.c|  7 ++-
 src/qemu/qemu_tpm.c   | 33 +++
 src/security/virt-aa-helper.c |  4 +---
 src/util/virfile.c|  6 +-
 src/util/virstoragefile.c | 13 ++--
 src/vmware/vmware_conf.c  |  7 ++-
 8 files changed, 13 insertions(+), 67 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 9b206abadc..4cf5b6e3d7 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -30,7 +30,6 @@
 #include "virstring.h"
 #include "node_device_conf.h"
 #include "device_conf.h"
-#include "dirname.h"
 #include "virxml.h"
 #include "virbuffer.h"
 #include "viruuid.h"
@@ -2445,8 +2444,7 @@ virNodeDeviceGetSCSITargetCaps(const char *sysfsPath,
 VIR_DEBUG("Checking if '%s' is an FC remote port", scsi_target->name);
 
 /* /sys/devices/[...]/host0/rport-0:0-0/target0:0:0 -> rport-0:0-0 */
-if (!(dir = mdir_name(sysfsPath)))
-return -1;
+dir = g_path_get_dirname(sysfsPath);
 
 rport = g_path_get_basename(dir);
 
diff --git a/src/locking/lock_driver_sanlock.c 
b/src/locking/lock_driver_sanlock.c
index c8936c301c..088255a111 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -31,7 +31,6 @@
 #include 
 #include 
 
-#include "dirname.h"
 #include "lock_driver.h"
 #include "virlog.h"
 #include "virerror.h"
@@ -239,10 +238,7 @@ 
virLockManagerSanlockSetupLockspace(virLockManagerSanlockDriverPtr driver)
 int perms = 0600;
 VIR_DEBUG("Lockspace %s does not yet exist", path);
 
-if (!(dir = mdir_name(path))) {
-virReportOOMError();
-goto error;
-}
+dir = g_path_get_dirname(path);
 if (stat(dir, ) < 0 || !S_ISDIR(st.st_mode)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to create lockspace %s: parent directory"
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec8faf384c..9dffeefce7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -99,7 +99,6 @@
 #include "vircgroup.h"
 #include "virperf.h"
 #include "virnuma.h"
-#include "dirname.h"
 #include "netdev_bandwidth_conf.h"
 #include "virqemu.h"
 #include "virdomainsnapshotobjlist.h"
@@ -850,10 +849,8 @@ qemuStateInitialize(bool privileged,
  (int)cfg->group);
 goto error;
 }
-if (!(channeldir = mdir_name(cfg->channelTargetDir))) {
-virReportOOMError();
-goto error;
-}
+channeldir = g_path_get_dirname(cfg->channelTargetDir);
+
 if (chown(channeldir, cfg->user, cfg->group) < 0) {
 virReportSystemError(errno,
  _("unable to set ownership of '%s' to %d:%d"),
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 91ab5bca9e..28800a100c 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -40,7 +40,6 @@
 #include "virstring.h"
 #include "virpidfile.h"
 #include "configmake.h"
-#include "dirname.h"
 #include "qemu_tpm.h"
 #include "virtpm.h"
 #include "secret_util.h"
@@ -83,26 +82,6 @@ qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir,
 }
 
 
-/*
- * virtTPMGetTPMStorageDir:
- *
- * @storagepath: directory for swtpm's persistent state
- *
- * Derive the 'TPMStorageDir' from the storagepath by searching
- * for the last '/'.
- */
-static char *
-qemuTPMGetTPMStorageDir(const char *storagepath)
-{
-char *ret = mdir_name(storagepath);
-
-if (!ret)
-virReportOOMError();
-
-return ret;
-}
-
-
 /*
  * qemuTPMEmulatorInitStorage
  *
@@ -147,10 +126,7 @@ qemuTPMCreateEmulatorStorage(const char *storagepath,
  gid_t swtpm_group)
 {
 int ret = -1;
-char *swtpmStorageDir = qemuTPMGetTPMStorageDir(storagepath);
-
-if (!swtpmStorageDir)
-return -1;
+char *swtpmStorageDir = g_path_get_dirname(storagepath);
 
 if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0)
 goto cleanup;
@@ -183,12 +159,9 @@ qemuTPMCreateEmulatorStorage(const char *storagepath,
 static void
 qemuTPMDeleteEmulatorStorage(virDomainTPMDefPtr tpm)
 {
-char *path = qemuTPMGetTPMStorageDir(tpm->data.emulator.storagepath);
+g_autofree char *path =  
g_path_get_dirname(tpm->data.emulator.storagepath);
 
-if (path) {
-ignore_value(virFileDeleteTree(path));
-VIR_FREE(path);
-}
+ignore_value(virFileDeleteTree(path));
 }
 
 
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index feb03b0aa9..9c34ac92c2 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -33,7 +33,6 @@
 #include "viralloc.h"
 #include "vircommand.h"
 #include "virlog.h"
-#include "dirname.h"
 #include "driver.h"
 
 #include 

[libvirt] [PATCH 05/23] src: always pull in glib/gstdio.h header

[Daniel P . Berrangé]

The gstdio.h header defines some low level wrappers for
things like fsync, stat, lstat, etc.

Signed-off-by: Daniel P. Berrangé 
---
 src/internal.h| 4 +---
 src/util/glibcompat.h | 1 +
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index 780f425d2c..686b7cfcc2 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -28,7 +28,7 @@
 #include 
 #include 
 #include 
-#include 
+#include "glibcompat.h"
 
 #if STATIC_ANALYSIS
 # undef NDEBUG /* Don't let a prior NDEBUG definition cause trouble.  */
@@ -63,8 +63,6 @@
 #include "libvirt/libvirt-admin.h"
 #include "libvirt/virterror.h"
 
-#include "glibcompat.h"
-
 /* Merely casting to (void) is not sufficient since the
  * introduction of the "warn_unused_result" attribute
  */
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index 9c5fef09bf..2bbbe57612 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -19,6 +19,7 @@
 #pragma once
 
 #include 
+#include 
 
 char *vir_g_strdup_printf(const char *msg, ...)
 G_GNUC_PRINTF(1, 2);
-- 
2.24.1

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

[libvirt] [PATCH 10/23] src: use g_lstat() instead of lstat()

[Daniel P . Berrangé]

The GLib g_lstat() function provides a portable impl for
Win32.

Signed-off-by: Daniel P. Berrangé 
---
 src/qemu/qemu_domain.c  |  8 
 src/security/security_dac.c |  2 +-
 src/storage/storage_util.c  |  2 +-
 src/util/vircgroupv1.c  |  4 ++--
 src/util/virfile.c  | 12 ++--
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ff87720fd1..187aaf5737 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -14232,7 +14232,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
 {
 char *devicePath = NULL;
 char *target = NULL;
-struct stat sb;
+GStatBuf sb;
 int ret = -1;
 bool isLink = false;
 bool isDev = false;
@@ -14250,7 +14250,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
 return ret;
 }
 
-if (lstat(device, ) < 0) {
+if (g_lstat(device, ) < 0) {
 if (errno == ENOENT && allow_noent) {
 /* Ignore non-existent device. */
 return 0;
@@ -15106,7 +15106,7 @@ struct qemuDomainAttachDeviceMknodData {
 virDomainObjPtr vm;
 const char *file;
 const char *target;
-struct stat sb;
+GStatBuf sb;
 void *acl;
 #ifdef WITH_SELINUX
 char *tcon;
@@ -15284,7 +15284,7 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr 
driver,
 data.vm = vm;
 data.file = file;
 
-if (lstat(file, ) < 0) {
+if (g_lstat(file, ) < 0) {
 virReportSystemError(errno,
  _("Unable to access %s"), file);
 return ret;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 411aa1b159..ccd3874897 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -2365,7 +2365,7 @@ virSecurityDACGetProcessLabelInternal(pid_t pid,
 
 path = g_strdup_printf("/proc/%d", (int)pid);
 
-if (lstat(path, ) < 0) {
+if (g_lstat(path, ) < 0) {
 virReportSystemError(errno,
  _("unable to get uid and gid for PID %d via 
procfs"),
  pid);
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 45ffd2206e..58225c09f5 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1522,7 +1522,7 @@ virStorageBackendVolOpen(const char *path, struct stat 
*sb,
 char *base = last_component(path);
 bool noerror = (flags & VIR_STORAGE_VOL_OPEN_NOERROR);
 
-if (lstat(path, sb) < 0) {
+if (g_lstat(path, sb) < 0) {
 if (errno == ENOENT) {
 if (noerror) {
 VIR_WARN("ignoring missing file '%s'", path);
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 4f6b89cf2e..d2ec7106db 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -223,7 +223,7 @@ virCgroupV1ResolveMountLink(const char *mntDir,
 g_autofree char *linkSrc = NULL;
 g_autofree char *tmp = NULL;
 char *dirName;
-struct stat sb;
+GStatBuf sb;
 
 tmp = g_strdup(mntDir);
 
@@ -241,7 +241,7 @@ virCgroupV1ResolveMountLink(const char *mntDir,
 linkSrc = g_strdup_printf("%s/%s", tmp, typeStr);
 *dirName = '/';
 
-if (lstat(linkSrc, ) < 0) {
+if (g_lstat(linkSrc, ) < 0) {
 if (errno == ENOENT) {
 VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s",
  typeStr, tmp, linkSrc);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index c79fe86403..97f86a46e9 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1002,11 +1002,11 @@ int virFileDeleteTree(const char *dir)
 
 while ((direrr = virDirRead(dh, , dir)) > 0) {
 g_autofree char *filepath = NULL;
-struct stat sb;
+GStatBuf sb;
 
 filepath = g_strdup_printf("%s/%s", dir, de->d_name);
 
-if (lstat(filepath, ) < 0) {
+if (g_lstat(filepath, ) < 0) {
 virReportSystemError(errno, _("Cannot access '%s'"),
  filepath);
 goto cleanup;
@@ -1555,7 +1555,7 @@ virFileResolveLinkHelper(const char *linkpath,
  bool intermediatePaths,
  char **resultpath)
 {
-struct stat st;
+GStatBuf st;
 
 *resultpath = NULL;
 
@@ -1563,7 +1563,7 @@ virFileResolveLinkHelper(const char *linkpath,
  * directories, if linkpath is absolute and the basename is
  * already a non-symlink.  */
 if (IS_ABSOLUTE_FILE_NAME(linkpath) && !intermediatePaths) {
-if (lstat(linkpath, ) < 0)
+if (g_lstat(linkpath, ) < 0)
 return -1;
 
 if (!S_ISLNK(st.st_mode)) {
@@ -1613,9 +1613,9 @@ virFileResolveAllLinks(const char *linkpath, char 
**resultpath)
 int
 virFileIsLink(const char *linkpath)
 {
-struct stat st;
+GStatBuf st;
 
-if (lstat(linkpath, ) < 0)
+if (g_lstat(linkpath, ) < 0)
 return -errno;
 
 return S_ISLNK(st.st_mode) != 0;
-- 
2.24.1

--
libvir-list mailing list

[libvirt] [PATCH 02/23] docs: expand macOS platform support coverage

[Daniel P . Berrangé]

We initially claimed to only support the most recent macOS
release, which is currently 10.15. Our Travis CI, however,
is validating 10.14.4 / XCode 10.3.

For almost all of our other platforms, we support multiple
releases to some degree. This change brings macOS in line
with other long life distros, covering the most recent &
most recent but one for a 2 year overlap. With this docs
change our CI is now actually testing our minimum version.

Signed-off-by: Daniel P. Berrangé 
---
 docs/platforms.html.in | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/docs/platforms.html.in b/docs/platforms.html.in
index a037074e1f..5c90f1dd7a 100644
--- a/docs/platforms.html.in
+++ b/docs/platforms.html.in
@@ -74,8 +74,17 @@
 macOS
 
 
-  The project supports building with the current version of macOS,
-  with the current homebrew package set available.
+  The project aims to support the most recent major version
+  at all times. Support for the previous major version will
+  be dropped 2 years after the new major version is released.
+
+
+
+  Note that to compile libvirt will require extra packages
+  to be made available on the macOS host. It is recommended
+  to use https://brew.sh/;>HomeBrew since this
+  is what libvirt CI tests with, however, https://www.macports.org/;>MacPorts
+  is an alternative option that is likely to work.
 
 
 FreeBSD
-- 
2.24.1

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

[libvirt] [PATCH 08/23] src: use g_fsync for portability

[Daniel P . Berrangé]

The g_fsync() API provides the same Windows portability
as GNULIB does for fsync().

Signed-off-by: Daniel P. Berrangé 
---
 src/nwfilter/nwfilter_dhcpsnoop.c | 2 +-
 src/storage/storage_util.c| 2 +-
 src/util/virfile.c| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index 9514dfd75c..9a71d13d57 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1772,7 +1772,7 @@ virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey,
 goto cleanup;
 }
 
-ignore_value(fsync(lfd));
+ignore_value(g_fsync(lfd));
 
  cleanup:
 VIR_FREE(lbuf);
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index f2d8810813..c1a6b44f4b 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -384,7 +384,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 }
 }
 
-if (fsync(fd) < 0) {
+if (g_fsync(fd) < 0) {
 ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 0f0d607c59..1784895575 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -519,7 +519,7 @@ virFileRewrite(const char *path,
 goto cleanup;
 }
 
-if (fsync(fd) < 0) {
+if (g_fsync(fd) < 0) {
 virReportSystemError(errno, _("cannot sync file '%s'"),
  newfile);
 goto cleanup;
-- 
2.24.1

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

[libvirt] [PATCH 07/23] util: add compat wrapper for g_fsync

[Daniel P . Berrangé]

g_fsync isn't available until 2.63 so we need a compat
wrapper temporarily.

Signed-off-by: Daniel P. Berrangé 
---
 src/libvirt_private.syms |  1 +
 src/util/glibcompat.c| 13 +
 src/util/glibcompat.h|  5 +
 3 files changed, 19 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a7b1ef23bc..d3184dbf5c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1504,6 +1504,7 @@ virSecurityManagerVerify;
 
 
 # util/glibcompat.h
+vir_g_fsync;
 vir_g_strdup_printf;
 vir_g_strdup_vprintf;
 
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 3f830840cf..4ebefb4478 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -19,11 +19,13 @@
 #include 
 
 #include 
+#include 
 
 #include "glibcompat.h"
 
 #undef g_strdup_printf
 #undef g_strdup_vprintf
+#undef g_fsync
 
 /* Due to a bug in glib, g_strdup_printf() nor g_strdup_vprintf()
  * abort on OOM.  It's fixed in glib's upstream. Provide our own
@@ -51,3 +53,14 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
 abort();
   return ret;
 }
+
+
+gint
+vir_g_fsync(gint fd)
+{
+#ifdef G_OS_WIN32
+  return _commit(fd);
+#else
+  return fsync(fd);
+#endif
+}
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index 2bbbe57612..d6b83f4b93 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -25,8 +25,13 @@ char *vir_g_strdup_printf(const char *msg, ...)
 G_GNUC_PRINTF(1, 2);
 char *vir_g_strdup_vprintf(const char *msg, va_list args)
 G_GNUC_PRINTF(1, 0);
+gint vir_g_fsync(gint fd);
 
 #if !GLIB_CHECK_VERSION(2, 64, 0)
 # define g_strdup_printf vir_g_strdup_printf
 # define g_strdup_vprintf vir_g_strdup_vprintf
 #endif
+
+#if !GLIB_CHECK_VERSION(2, 63, 0)
+# define g_fsync vir_g_fsync
+#endif
-- 
2.24.1

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

Re: [libvirt] [PATCH] qemu: Warn of restore with managed save being risky

[Daniel Henrique Barboza]

On 1/2/20 11:36 AM, Michael Weiser wrote:

Hello Daniel,

On Thu, Jan 02, 2020 at 09:58:19AM -0300, Daniel Henrique Barboza wrote:



[...]

 

I'd rather not reference virsh command options in the error message as
it would be highly confusing in any other context. For example, python
clients get the error message wrapped in an exception, augmented already
by a prefix telling them they need to force the operation:


Good point.
 


Traceback (most recent call last):
   File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in 
cb_wrapper
 callback(asyncjob, *args, **kwargs)
   File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
 callback(*args, **kwargs)
   File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line 66, 
in newfn
 ret = fn(self, *args, **kwargs)
   File "/usr/share/virt-manager/virtManager/object/domain.py", line 1055, in 
revert_to_snapshot
 self._backend.revertToSnapshot(snap.get_backend())
   File "/usr/lib64/python3.6/site-packages/libvirt.py", line 2088, in 
revertToSnapshot
 if ret == -1: raise libvirtError ('virDomainRevertToSnapshot() failed', 
dom=self)
libvirt.libvirtError: revert requires force: revert to snapshot while
there is a managed saved state will cause corruption when run, remove
saved state first

The same is actually the case for virsh already:

virsh # snapshot-revert debian --snapshotname snapshot1
error: revert requires force: revert to snapshot while there is a
managed saved state will cause corruption when run, remove saved state
first

virsh #

We could of course reword to better take context and prefix into
account, e.g.:


Since there is already a "revert requires force" prefix in both python and
virsh error messages, changing the error message of this v1 becomes more of
a wording/flavor issue.



error: revert requires force: Removal of existing managed saved state
strongly recommended to avoid corruption



I prefer this wording though :)


Thanks,


DHB

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

[libvirt] [PATCH] travis: fix homebrew install of python3

[Daniel P . Berrangé]

The Python3 package has started failing to install from
HomeBrew with the following:

  Error: The `brew link` step did not complete successfully
  The formula built, but is not symlinked into /usr/local
  Could not symlink Frameworks/Python.framework/Headers
  Target /usr/local/Frameworks/Python.framework/Headers
  is a symlink belonging to python@2. You can unlink it:

brew unlink python@2

  To force the link and overwrite all conflicting files:

brew link --overwrite python

The result is that libvirt fails to find python3:

  checking for python3... no
  configure: error: 'python3' binary is required to build libvirt

It is unclear what changed in Travis/HomeBrew to break our
previously working setup, but running the suggested command
fixes it well enough for libvirt's CI needs.

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

Pushed as a CI build fix

 .travis.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 44ff5e9898..f129706456 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -67,6 +67,9 @@ matrix:
   env:
 - 
PATH="/usr/local/opt/gettext/bin:/usr/local/opt/ccache/libexec:/usr/local/opt/rpcgen/bin:$PATH"
 - PKG_CONFIG_PATH="/usr/local/opt/libxml2/lib/pkgconfig"
+  before_script:
+# Hack to blow away py2
+- brew link --overwrite python
   script:
 # We can't run 'distcheck' or 'syntax-check' because they fail on
 # macOS, but doing 'install' and 'dist' gives us some useful coverage
-- 
2.24.1

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

Re: [libvirt] [PATCH] qemu: Warn of restore with managed save being risky

[Michael Weiser]

Hello Daniel,

On Thu, Jan 02, 2020 at 09:58:19AM -0300, Daniel Henrique Barboza wrote:

> > > > This patch marks the restore operation as risky at the libvirt level,
> > > > requiring the user to remove the saved memory state first or force the
> > > > operation.
> > > > 
> > > > [1] 
> > > > https://www.redhat.com/archives/virt-tools-list/2019-November/msg00011.html
> > > > [2] 
> > > > https://www.redhat.com/archives/virt-tools-list/2019-December/msg00049.html
> > > > 
> > > > Signed-off-by: Michael Weiser 
> > > > Cc: Cole Robinson 
> > > > ---
> > > As said in [1], I agree that the API needs a flag override to allow the 
> > > user
> > > to roll with it despite the risks. Given that this is somewhat a corner 
> > > case,
> > > I also believe that such override can go in a separated patch/series later
> > > on.
> > 
> > Do you mean a separate override flag beyond
> > VIR_DOMAIN_SNAPSHOT_REVERT_FORCE? Or should I just update the commit
> > message to make clear how the user can force the operation?
> Ah, I overlooked the "or force the operation" part of the commit message after
> reading the discussions in [1] and [2].

> Instead of updating the commit message, I think you can update the error 
> message
> to mention the '--force' option, e.g. :


> +virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
> +   _("revert to snapshot while there is a managed "
> + "saved state will cause corruption when run, "
> + "remove saved state first or use the "
> + "--force option"));

I'd rather not reference virsh command options in the error message as
it would be highly confusing in any other context. For example, python
clients get the error message wrapped in an exception, augmented already
by a prefix telling them they need to force the operation:

Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in cb_wrapper
callback(asyncjob, *args, **kwargs)
  File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
callback(*args, **kwargs)
  File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line 66, 
in newfn
ret = fn(self, *args, **kwargs)
  File "/usr/share/virt-manager/virtManager/object/domain.py", line 1055, in 
revert_to_snapshot
self._backend.revertToSnapshot(snap.get_backend())
  File "/usr/lib64/python3.6/site-packages/libvirt.py", line 2088, in 
revertToSnapshot
if ret == -1: raise libvirtError ('virDomainRevertToSnapshot() failed', 
dom=self)
libvirt.libvirtError: revert requires force: revert to snapshot while
there is a managed saved state will cause corruption when run, remove
saved state first

The same is actually the case for virsh already:

virsh # snapshot-revert debian --snapshotname snapshot1
error: revert requires force: revert to snapshot while there is a
managed saved state will cause corruption when run, remove saved state
first

virsh # 

We could of course reword to better take context and prefix into
account, e.g.:

error: revert requires force: Removal of existing managed saved state
strongly recommended to avoid corruption

Any ideas?

> I also noticed that the man page for 'snapshot-revert' mentions two cases 
> where
> the use of --force is required. One of them talks about snapshots created 
> using old
> Libvirt versions. The other goes as follows:

> 
> (docs/manpages/virsh.rst)


> The other is the case of reverting from a running domain to an active state
> where a new hypervisor has to be created rather than reusing the existing
> hypervisor, because it implies drawbacks such as breaking any existing
> VNC or Spice connections; this condition happens with an active
> snapshot that uses a provably incompatible configuration, as well as
> with an inactive snapshot that is combined with the --start or
> --pause flag.
> 

> I am almost certain that scenario you're handling here is not covered by this
> excerpt. In this case, since you're adding a new '--force' condition, I 
> believe a
> second patch adding this new '--force' condition in the documentation is a
> nice touch.

Will do. v2 coming as soon as we've agreed on where to go with the error 
message.
-- 
Thanks,
Michael


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

Re: [libvirt] [PATCH 5/7] util: Add ADD_ARG_RETURN_ON_ERROR

[Fabiano Fidêncio]

[snip]

> > +#define ADD_ARG_RETURN_ON_ERROR(rule, str) \
> > +do { \
> > +if (VIR_RESIZE_N(rule->args, \
> > + rule->argsAlloc, \
> > + rule->argsLen, 1) < 0) \
> > +return; \
> > + \
> > +rule->args[rule->argsLen++] = g_strdup(str); \
> > +} while (0)
>
> IMHO this is missing the benefit of using glib since VIR_RESIZE_N
> will never fail now. We should get rid of "ADD_ARG" entirely, and
> just put a g_realloc() call directly in the place it is needed
> without any macros. Likewise for the next patch.

Cool, I really missed that.
I'll submit a v2, soon.

Best Regards,
-- 
Fabiano Fidêncio.


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

Re: [libvirt] [PATCH 5/7] util: Add ADD_ARG_RETURN_ON_ERROR

[Daniel P . Berrangé]

On Thu, Jan 02, 2020 at 02:57:56PM +0100, Fabiano Fidêncio wrote:
> Similarly to ADD_ARG, let's create an ADD_ARG_RETURN_ON_ERROR macro
> which will simply return instead of going to a cleanup label.
> 
> By doing this, we can get rid of a few cleanup labels spread in the
> code.
> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  src/util/virfirewall.c | 25 +
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
> index 6dace18bc4..d632f72502 100644
> --- a/src/util/virfirewall.c
> +++ b/src/util/virfirewall.c
> @@ -328,6 +328,16 @@ void virFirewallFree(virFirewallPtr firewall)
>  rule->args[rule->argsLen++] = g_strdup(str); \
>  } while (0)
>  
> +#define ADD_ARG_RETURN_ON_ERROR(rule, str) \
> +do { \
> +if (VIR_RESIZE_N(rule->args, \
> + rule->argsAlloc, \
> + rule->argsLen, 1) < 0) \
> +return; \
> + \
> +rule->args[rule->argsLen++] = g_strdup(str); \
> +} while (0)

IMHO this is missing the benefit of using glib since VIR_RESIZE_N
will never fail now. We should get rid of "ADD_ARG" entirely, and
just put a g_realloc() call directly in the place it is needed
without any macros. Likewise for the next patch.


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

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

[libvirt] [PATCH 7/7] docs: Remove mention to no_memory label

[Fabiano Fidêncio]

no_memory labels have been entirely removed from libvirt codebase.
Knowing that, let's also remove any mention to the label from our
hacking guide.

Signed-off-by: Fabiano Fidêncio 
---
 docs/hacking.html.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 74aba5d46b..90bd0ddc81 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1491,7 +1491,6 @@ BAD:
 
   error: A path only taken upon return with an error code
 cleanup: A path taken upon return with success code + optional error
-  no_memory: A path only taken upon return with an OOM error code
   retry: If needing to jump upwards (e.g., retry on EINTR)
 
 
-- 
2.24.1

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

[libvirt] [PATCH 4/7] util: Use g_auto in virFirewallStartTransaction()

[Fabiano Fidêncio]

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index d6fff31318..6dace18bc4 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -278,6 +278,7 @@ virFirewallGroupFree(virFirewallGroupPtr group)
 VIR_FREE(group);
 }
 
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallGroupPtr, virFirewallGroupFree, 
NULL);
 
 /**
  * virFirewallFree:
@@ -575,7 +576,7 @@ size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule)
 void virFirewallStartTransaction(virFirewallPtr firewall,
  unsigned int flags)
 {
-virFirewallGroupPtr group;
+g_auto(virFirewallGroupPtr) group = NULL;
 
 VIR_FIREWALL_RETURN_IF_ERROR(firewall);
 
@@ -586,10 +587,9 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
 
 if (VIR_EXPAND_N(firewall->groups,
  firewall->ngroups, 1) < 0) {
-virFirewallGroupFree(group);
 return;
 }
-firewall->groups[firewall->ngroups - 1] = group;
+firewall->groups[firewall->ngroups - 1] = g_steal_pointer();
 firewall->currentGroup = firewall->ngroups - 1;
 }
 
-- 
2.24.1

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

[libvirt] [PATCH 1/7] util: Rename 'no_memory' label to 'cleanup'

[Fabiano Fidêncio]

Let's rename the no_memory label to cleanup as the first step to remove
the OOM handling in virfirewall.c file.

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index ee72b579e4..c6a5de8842 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -321,7 +321,7 @@ void virFirewallFree(virFirewallPtr firewall)
 if (VIR_RESIZE_N(rule->args, \
  rule->argsAlloc, \
  rule->argsLen, 1) < 0) \
-goto no_memory; \
+goto cleanup; \
  \
 rule->args[rule->argsLen++] = g_strdup(str); \
 } while (0)
@@ -348,7 +348,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 
 
 if (VIR_ALLOC(rule) < 0)
-goto no_memory;
+goto cleanup;
 
 rule->layer = layer;
 rule->queryCB = cb;
@@ -379,18 +379,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 if (VIR_APPEND_ELEMENT_COPY(group->rollback,
 group->nrollback,
 rule) < 0)
-goto no_memory;
+goto cleanup;
 } else {
 if (VIR_APPEND_ELEMENT_COPY(group->action,
 group->naction,
 rule) < 0)
-goto no_memory;
+goto cleanup;
 }
 
 
 return rule;
 
- no_memory:
+ cleanup:
 firewall->err = ENOMEM;
 virFirewallRuleFree(rule);
 return NULL;
@@ -494,7 +494,7 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
 
 return;
 
- no_memory:
+ cleanup:
 firewall->err = ENOMEM;
 }
 
@@ -516,7 +516,7 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
 
 return;
 
- no_memory:
+ cleanup:
 firewall->err = ENOMEM;
 }
 
@@ -534,7 +534,7 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
 
 return;
 
- no_memory:
+ cleanup:
 firewall->err = ENOMEM;
 }
 
@@ -557,7 +557,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
 
 return;
 
- no_memory:
+ cleanup:
 firewall->err = ENOMEM;
 va_end(list);
 }
-- 
2.24.1

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

[libvirt] [PATCH 5/7] util: Add ADD_ARG_RETURN_ON_ERROR

[Fabiano Fidêncio]

Similarly to ADD_ARG, let's create an ADD_ARG_RETURN_ON_ERROR macro
which will simply return instead of going to a cleanup label.

By doing this, we can get rid of a few cleanup labels spread in the
code.

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 6dace18bc4..d632f72502 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -328,6 +328,16 @@ void virFirewallFree(virFirewallPtr firewall)
 rule->args[rule->argsLen++] = g_strdup(str); \
 } while (0)
 
+#define ADD_ARG_RETURN_ON_ERROR(rule, str) \
+do { \
+if (VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1) < 0) \
+return; \
+ \
+rule->args[rule->argsLen++] = g_strdup(str); \
+} while (0)
+
 static virFirewallRulePtr
 virFirewallAddRuleFullV(virFirewallPtr firewall,
 virFirewallLayer layer,
@@ -490,10 +500,7 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
 {
 VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
 
-ADD_ARG(rule, arg);
-
- cleanup:
-return;
+ADD_ARG_RETURN_ON_ERROR(rule, arg);
 }
 
 
@@ -510,10 +517,7 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
 arg = g_strdup_vprintf(fmt, list);
 va_end(list);
 
-ADD_ARG(rule, arg);
-
- cleanup:
-return;
+ADD_ARG_RETURN_ON_ERROR(rule, arg);
 }
 
 
@@ -524,12 +528,9 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
 VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
 
 while (*args) {
-ADD_ARG(rule, *args);
+ADD_ARG_RETURN_ON_ERROR(rule, *args);
 args++;
 }
-
- cleanup:
-return;
 }
 
 
-- 
2.24.1

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

[libvirt] [PATCH 2/7] util: Don't set/check for ENOMEM as a firewall error

[Fabiano Fidêncio]

As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of OOM treatment in virfirewall.c and
simplify the code whenever it's possible.

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 30 ++
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index c6a5de8842..00b1bd0a97 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -391,7 +391,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 return rule;
 
  cleanup:
-firewall->err = ENOMEM;
 virFirewallRuleFree(rule);
 return NULL;
 }
@@ -492,10 +491,8 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
 
 ADD_ARG(rule, arg);
 
-return;
-
  cleanup:
-firewall->err = ENOMEM;
+return;
 }
 
 
@@ -514,10 +511,8 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
 
 ADD_ARG(rule, arg);
 
-return;
-
  cleanup:
-firewall->err = ENOMEM;
+return;
 }
 
 
@@ -532,10 +527,8 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
 args++;
 }
 
-return;
-
  cleanup:
-firewall->err = ENOMEM;
+return;
 }
 
 
@@ -553,12 +546,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
 while ((str = va_arg(list, char *)) != NULL)
 ADD_ARG(rule, str);
 
-va_end(list);
-
-return;
-
  cleanup:
-firewall->err = ENOMEM;
 va_end(list);
 }
 
@@ -591,15 +579,13 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
 
 VIR_FIREWALL_RETURN_IF_ERROR(firewall);
 
-if (!(group = virFirewallGroupNew())) {
-firewall->err = ENOMEM;
+if (!(group = virFirewallGroupNew()))
 return;
-}
+
 group->actionFlags = flags;
 
 if (VIR_EXPAND_N(firewall->groups,
  firewall->ngroups, 1) < 0) {
-firewall->err = ENOMEM;
 virFirewallGroupFree(group);
 return;
 }
@@ -747,10 +733,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
 if (rule->queryCB(firewall, rule->layer, (const char *const *)lines, 
rule->queryOpaque) < 0)
 return -1;
 
-if (firewall->err == ENOMEM) {
-virReportOOMError();
-return -1;
-}
 if (firewall->err) {
 virReportSystemError(firewall->err, "%s",
  _("Unable to create rule"));
@@ -818,7 +800,7 @@ virFirewallApply(virFirewallPtr firewall)
_("Failed to initialize a valid firewall backend"));
 goto cleanup;
 }
-if (!firewall || firewall->err == ENOMEM) {
+if (!firewall) {
 virReportOOMError();
 goto cleanup;
 }
-- 
2.24.1

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

[libvirt] [PATCH 3/7] util: Use g_auto/g_autofree in virFirewallAddRuleFullV()

[Fabiano Fidêncio]

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 00b1bd0a97..d6fff31318 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -257,6 +257,7 @@ virFirewallRuleFree(virFirewallRulePtr rule)
 VIR_FREE(rule);
 }
 
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallRulePtr, virFirewallRuleFree, NULL);
 
 static void
 virFirewallGroupFree(virFirewallGroupPtr group)
@@ -335,8 +336,8 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 va_list args)
 {
 virFirewallGroupPtr group;
-virFirewallRulePtr rule;
-char *str;
+g_auto(virFirewallRulePtr) rule = NULL;
+g_autofree char *str = NULL;
 
 VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall);
 
@@ -348,7 +349,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 
 
 if (VIR_ALLOC(rule) < 0)
-goto cleanup;
+return NULL;
 
 rule->layer = layer;
 rule->queryCB = cb;
@@ -379,19 +380,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 if (VIR_APPEND_ELEMENT_COPY(group->rollback,
 group->nrollback,
 rule) < 0)
-goto cleanup;
+return NULL;
 } else {
 if (VIR_APPEND_ELEMENT_COPY(group->action,
 group->naction,
 rule) < 0)
-goto cleanup;
+return NULL;
 }
 
 
-return rule;
+return g_steal_pointer();
 
  cleanup:
-virFirewallRuleFree(rule);
 return NULL;
 }
 
-- 
2.24.1

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

[libvirt] [PATCH 0/7] Remove the last "no_memory" label

[Fabiano Fidêncio]

This series touches virfirewall.c, the last place where a no_memory
label can be found.

The series:
- Renames no_memory to cleanup;
- Gets rid of ENOMEM error treatment;
- Use g_auto / g_autofree in a few different places;
- Add new macros as ADD_ARG heavily rely on a goto;
- Remove no_memory mention from the hacking guide;

I'm not totally sure whether the new macros addition are worth it.

Fabiano Fidêncio (7):
  util: Rename 'no_memory' label to 'cleanup'
  util: Don't set/check for ENOMEM as a firewall error
  util: Use g_auto/g_autofree in virFirewallAddRuleFullV()
  util: Use g_auto in virFirewallStartTransaction()
  util: Add ADD_ARG_RETURN_ON_ERROR
  util: Add ADD_ARG_RETURN_VALUE_ON_ERROR
  docs: Remove mention to no_memory label

 docs/hacking.html.in   |  1 -
 src/util/virfirewall.c | 94 +++---
 2 files changed, 42 insertions(+), 53 deletions(-)

-- 
2.24.1

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

[libvirt] [PATCH 6/7] util: Add ADD_ARG_RETURN_VALUE_ON_ERROR

[Fabiano Fidêncio]

Similarly to ADD_ARG, let's create an ADD_ARG_RETURN_VALUE_ON_ERROR
macro which will simply return a value instead of going to a cleanup
label.

This patch will get rid of the cleanup label present in
virFirewallAddRuleFullV().

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virfirewall.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index d632f72502..1b04fc5ee4 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -338,6 +338,16 @@ void virFirewallFree(virFirewallPtr firewall)
 rule->args[rule->argsLen++] = g_strdup(str); \
 } while (0)
 
+#define ADD_ARG_RETURN_VALUE_ON_ERROR(rule, str, value) \
+do { \
+if (VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1) < 0) \
+return value; \
+ \
+rule->args[rule->argsLen++] = g_strdup(str); \
+} while (0)
+
 static virFirewallRulePtr
 virFirewallAddRuleFullV(virFirewallPtr firewall,
 virFirewallLayer layer,
@@ -370,22 +380,22 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 switch (rule->layer) {
 case VIR_FIREWALL_LAYER_ETHERNET:
 if (ebtablesUseLock)
-ADD_ARG(rule, "--concurrent");
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, "--concurrent", NULL);
 break;
 case VIR_FIREWALL_LAYER_IPV4:
 if (iptablesUseLock)
-ADD_ARG(rule, "-w");
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, "-w", NULL);
 break;
 case VIR_FIREWALL_LAYER_IPV6:
 if (ip6tablesUseLock)
-ADD_ARG(rule, "-w");
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, "-w", NULL);
 break;
 case VIR_FIREWALL_LAYER_LAST:
 break;
 }
 
 while ((str = va_arg(args, char *)) != NULL)
-ADD_ARG(rule, str);
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, str, NULL);
 
 if (group->addingRollback) {
 if (VIR_APPEND_ELEMENT_COPY(group->rollback,
@@ -401,9 +411,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
 
 
 return g_steal_pointer();
-
- cleanup:
-return NULL;
 }
 
 
-- 
2.24.1

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

Re: [libvirt] [PATCH] qemu: Warn of restore with managed save being risky

[Daniel Henrique Barboza]

On 12/31/19 5:42 PM, Michael Weiser wrote:

Hi Daniel,

On Tue, Dec 31, 2019 at 05:19:54PM -0300, Daniel Henrique Barboza wrote:


This patch marks the restore operation as risky at the libvirt level,
requiring the user to remove the saved memory state first or force the
operation.

[1] https://www.redhat.com/archives/virt-tools-list/2019-November/msg00011.html
[2] https://www.redhat.com/archives/virt-tools-list/2019-December/msg00049.html

Signed-off-by: Michael Weiser 
Cc: Cole Robinson 
---

As said in [1], I agree that the API needs a flag override to allow the user
to roll with it despite the risks. Given that this is somewhat a corner case,
I also believe that such override can go in a separated patch/series later
on.


Do you mean a separate override flag beyond
VIR_DOMAIN_SNAPSHOT_REVERT_FORCE? Or should I just update the commit
message to make clear how the user can force the operation?



Ah, I overlooked the "or force the operation" part of the commit message after
reading the discussions in [1] and [2].

Instead of updating the commit message, I think you can update the error message
to mention the '--force' option, e.g. :


+virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
+   _("revert to snapshot while there is a managed "
+ "saved state will cause corruption when run, "
+ "remove saved state first or use the "
+ "--force option"));



I also noticed that the man page for 'snapshot-revert' mentions two cases where
the use of --force is required. One of them talks about snapshots created using 
old
Libvirt versions. The other goes as follows:


(docs/manpages/virsh.rst)


The other is the case of reverting from a running domain to an active state
where a new hypervisor has to be created rather than reusing the existing
hypervisor, because it implies drawbacks such as breaking any existing
VNC or Spice connections; this condition happens with an active
snapshot that uses a provably incompatible configuration, as well as
with an inactive snapshot that is combined with the --start or
--pause flag.


I am almost certain that scenario you're handling here is not covered by this
excerpt. In this case, since you're adding a new '--force' condition, I believe 
a
second patch adding this new '--force' condition in the documentation is a
nice touch.



Thanks,


DHB





Reviewed-by: Daniel Henrique Barboza 


Thanks!



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

[libvirt] [PATCHv2 0/2] Introduce the support of Intel RDT-MBM

[Wang Huaqiang]

This is a followup to Daniel's review in thread 
https://www.redhat.com/archives/libvir-list/2019-December/msg00978.html.

Hi Daniel,

In this series, patch 1/2 is trying to fix a bug that using the 64bit,
instead of the current 32bit, interface, to hold cache monitor value
reported by underlying 64bit counter. 
In last series, you have suggested
of adding a new field such as "cpu.cache.monitor.%zu.bank.%zu.bytes64"
for new 64bit counter result, and keep the old but already exported 32bit
field, to avoid reporting a truncated cache monitor result to user.
But considering the fact that, in reality, CPU cache size will never
exceed 4GB in size, it is safe to truccate a 64bit counter to 32 bit
since the counter is counting for the cache size that vcpus are using.

Change List:
v2:
* Using old "cpu.cache.monitor.%zu.bank.%zu.bytes" field for cache
monitor result, and keeping the interface consistent.
* Add 'virsh domstats --memory' document in file 'virsh.rst'

Wang Huaqiang (2):
  util, resctrl: using 64bit interface instead of 32bit for counters
  Introduce command 'virsh domstats --memory' for reporting memory BW

 docs/manpages/virsh.rst  |  22 +-
 include/libvirt/libvirt-domain.h |   1 +
 src/libvirt-domain.c |  21 ++
 src/qemu/qemu_driver.c   | 118 ++-
 src/util/virfile.c   |  40 +++
 src/util/virfile.h   |   2 +
 src/util/virresctrl.c|   6 +-
 src/util/virresctrl.h|   2 +-
 tools/virsh-domain-monitor.c |   7 ++
 9 files changed, 211 insertions(+), 8 deletions(-)

-- 
2.23.0


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

[libvirt] [PATCHv2 1/2] util, resctrl: using 64bit interface instead of 32bit for counters

[Wang Huaqiang]

The underlying resctrl monitoring is actually using 64 bit counters,
not the 32bit one. Correct this by using 64bit data type for reading
hardware value.

To keep the interface consistent, the result of CPU last level cache
that occupied by vcpu processors of specific restrl monitor group is
still reported with a truncated 32bit data type. because, in silicon
world, CPU cache size will never exceed 4GB.

Signed-off-by: Wang Huaqiang 
---
 src/qemu/qemu_driver.c | 15 +--
 src/util/virfile.c | 40 
 src/util/virfile.h |  2 ++
 src/util/virresctrl.c  |  6 +++---
 src/util/virresctrl.h  |  2 +-
 5 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a7f388767..ad6f3130ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20740,8 +20740,19 @@ qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver,
  "cpu.cache.monitor.%zu.bank.%zu.id", 
i, j) < 0)
 goto cleanup;
 
-if (virTypedParamListAddUInt(params, resdata[i]->stats[j]->vals[0],
- 
"cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0)
+/* 'resdata[i]->stats[j]->vals[0]' keeps the value of how many last
+ * level cache in bank j currently occupied by the vcpus listed in
+ * resource monitor i, in bytes. This value is reported through a
+ * 64 bit hardware counter, so it is better to be arranged with
+ * data type in 64 bit width, but considering the fact that
+ * physical cache on a CPU could never be designed to be bigger
+ * than 4G bytes in size, to keep the 'domstats' interface
+ * historically consistent, it is safe to report the value with a
+ * truncated 'UInt' data type here. */
+if (virTypedParamListAddUInt(params,
+ (unsigned 
int)resdata[i]->stats[j]->vals[0],
+ 
"cpu.cache.monitor.%zu.bank.%zu.bytes",
+ i, j) < 0)
 goto cleanup;
 }
 }
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 4fd865dd83..7bf33682c7 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4196,6 +4196,46 @@ virFileReadValueUint(unsigned int *value, const char 
*format, ...)
 }
 
 
+/**
+ * virFileReadValueUllong:
+ * @value: pointer to unsigned long long to be filled in with the value
+ * @format, ...: file to read from
+ *
+ * Read unsigned int from @format and put it into @value.
+ *
+ * Return -2 for non-existing file, -1 on other errors and 0 if everything went
+ * fine.
+ */
+int
+virFileReadValueUllong(unsigned long long *value, const char *format, ...)
+{
+g_autofree char *str = NULL;
+g_autofree char *path = NULL;
+va_list ap;
+
+va_start(ap, format);
+path = g_strdup_vprintf(format, ap);
+va_end(ap);
+
+if (!virFileExists(path))
+return -2;
+
+if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), ) < 0)
+return -1;
+
+virStringTrimOptionalNewline(str);
+
+if (virStrToLong_ullp(str, NULL, 10, value) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Invalid unsigned long long value '%s' in file '%s'"),
+   str, path);
+return -1;
+}
+
+return 0;
+}
+
+
 /**
  * virFileReadValueScaledInt:
  * @value: pointer to unsigned long long int to be filled in with the value
diff --git a/src/util/virfile.h b/src/util/virfile.h
index bcae40ee06..756c9a70b9 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -361,6 +361,8 @@ int virFileReadValueInt(int *value, const char *format, ...)
  G_GNUC_PRINTF(2, 3);
 int virFileReadValueUint(unsigned int *value, const char *format, ...)
  G_GNUC_PRINTF(2, 3);
+int virFileReadValueUllong(unsigned long long *value, const char *format, ...)
+ G_GNUC_PRINTF(2, 3);
 int virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...)
  G_GNUC_PRINTF(2, 3);
 int virFileReadValueScaledInt(unsigned long long *value, const char *format, 
...)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index b78fded026..684d2ce398 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2678,7 +2678,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
 int rv = -1;
 int ret = -1;
 size_t i = 0;
-unsigned int val = 0;
+unsigned long long val = 0;
 DIR *dirp = NULL;
 char *datapath = NULL;
 char *filepath = NULL;
@@ -2734,8 +2734,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
 goto cleanup;
 
 for (i = 0; resources[i]; i++) {
-rv = virFileReadValueUint(, "%s/%s/%s", datapath,
-  ent->d_name, resources[i]);
+rv = virFileReadValueUllong(, "%s/%s/%s", datapath,
+   

[libvirt] [PATCHv2 2/2] Introduce command 'virsh domstats --memory' for reporting memory BW

[Wang Huaqiang]

Introduce an option '--memory' for showing memory related
information. The memory bandwidth infomatio is listed as:

Domain: 'libvirt-vm'
 memory.bandwidth.monitor.count=4
 memory.bandwidth.monitor.0.name=vcpus_0-4
 memory.bandwidth.monitor.0.vcpus=0-4
 memory.bandwidth.monitor.0.node.count=2
 memory.bandwidth.monitor.0.node.0.id=0
 memory.bandwidth.monitor.0.node.0.bytes.total=10208067584
 memory.bandwidth.monitor.0.node.0.bytes.local=4807114752
 memory.bandwidth.monitor.0.node.1.id=1
 memory.bandwidth.monitor.0.node.1.bytes.total=8693735424
 memory.bandwidth.monitor.0.node.1.bytes.local=5850161152
 memory.bandwidth.monitor.1.name=vcpus_7
 memory.bandwidth.monitor.1.vcpus=7
 memory.bandwidth.monitor.1.node.count=2
 memory.bandwidth.monitor.1.node.0.id=0
 memory.bandwidth.monitor.1.node.0.bytes.total=853811200
 memory.bandwidth.monitor.1.node.0.bytes.local=290701312
 memory.bandwidth.monitor.1.node.1.id=1
 memory.bandwidth.monitor.1.node.1.bytes.total=406044672
 memory.bandwidth.monitor.1.node.1.bytes.local=229425152

Signed-off-by: Wang Huaqiang 
---
 docs/manpages/virsh.rst  |  22 ++-
 include/libvirt/libvirt-domain.h |   1 +
 src/libvirt-domain.c |  21 +++
 src/qemu/qemu_driver.c   | 103 +++
 tools/virsh-domain-monitor.c |   7 +++
 5 files changed, 152 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..d0f9e15c38 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2186,7 +2186,7 @@ domstats
 
domstats [--raw] [--enforce] [--backing] [--nowait] [--state]
   [--cpu-total] [--balloon] [--vcpu] [--interface]
-  [--block] [--perf] [--iothread]
+  [--block] [--perf] [--iothread] [--memory]
   [[--list-active] [--list-inactive]
[--list-persistent] [--list-transient] [--list-running]y
[--list-paused] [--list-shutoff] [--list-other]] | [domain ...]
@@ -2205,7 +2205,7 @@ behavior use the *--raw* flag.
 The individual statistics groups are selectable via specific flags. By
 default all supported statistics groups are returned. Supported
 statistics groups flags are: *--state*, *--cpu-total*, *--balloon*,
-*--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*.
+*--vcpu*, *--interface*, *--block*, *--perf*, *--iothread*, *--memory*.
 
 Note that - depending on the hypervisor type and version or the domain state
 - not all of the following statistics may be returned.
@@ -2372,6 +2372,24 @@ not available for statistical purposes.
 * ``iothread..poll-shrink`` - polling time shrink value. A value of
   (zero) indicates shrink is managed by hypervisor.
 
+*--memory* returns:
+
+* ``memory.bandwidth.monitor.count`` - the number of memory bandwidth
+  monitors for this domain
+* ``memory.bandwidth.monitor..name``  - the name of monitor 
+* ``memory.bandwidth.monitor..vcpus`` - the vcpu list of monitor 
+* ``memory.bandwidth.monitor..node.count`` - the number of memory
+controller in monitor 
+* ``memory.bandwidth.monitor..node..id`` - host allocated memory
+  controller id for controller  of monitor 
+* ``memory.bandwidth.monitor..node..bytes.local`` - the 
accumulative
+  bytes consumed by @vcpus that passing through the memory controller in the
+  same processor that the scheduled host CPU belongs to.
+* ``memory.bandwidth.monitor..node..bytes.total`` - the total
+  bytes consumed by @vcpus that passing through all memory controllers, either
+  local or remote controller.
+
+
 Selecting a specific statistics groups doesn't guarantee that the
 daemon supports the selected group of stats. Flag *--enforce*
 forces the command to fail if the daemon doesn't support the
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index e60003978a..78532c0eb8 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2160,6 +2160,7 @@ typedef enum {
 VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
 VIR_DOMAIN_STATS_PERF = (1 << 6), /* return domain perf event info */
 VIR_DOMAIN_STATS_IOTHREAD = (1 << 7), /* return iothread poll info */
+VIR_DOMAIN_STATS_MEMORY= (1 << 8), /* return domain memory info */
 } virDomainStatsTypes;
 
 typedef enum {
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 793eceb39f..eb66999f07 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11640,6 +11640,27 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * hypervisor to choose how to shrink the
  * polling time.
  *
+ * VIR_DOMAIN_STATS_MEMORY:
+ * Return memory bandwidth statistics and the usage information. The typed
+ * parameter keys are in this format:
+ *
+ * "memory.bandwidth.monitor.count" - the number of memory bandwidth
+ *monitors for this domain
+ * "memory.bandwidth.monitor..name" - the name of monitor 
+ * 

[libvirt] when libvirt xml define nic driver e1000, my vm fail to start

[thomas.kuang]

hi, all:
my xml  define interface as following:

  
  
  

when run the command:
bash-4.1# virsh start root-vsys_v77
error: Failed to start domain root-vsys_v77
error: Unable to open /dev/net/tun, is tun module loaded?: No such file or 
directory


is my xml define error ? how can create my vm with nic driver e1000 ?




bash-4.1# libvirtd -v,yum install libvirt
2020-01-02 10:00:49.261+: 8349: info : libvirt version: 4.5.0, package: 
23.el7 (CentOS BuildSystem , 2019-08-09-00:39:08, 
x86-02.bsys.centos.org)
2020-01-02 10:00:49.261+: 8349: info : hostname: NSG
2020-01-02 10:00:49.261+: 8349: warning : virGetHostnameImpl:719 : 
getaddrinfo failed for 'NSG': Name or service not known
2020-01-02 10:00:49.263+: 8349: info : libvirt version: 4.5.0, package: 
23.el7 (CentOS BuildSystem , 2019-08-09-00:39:08, 
x86-02.bsys.centos.org)
2020-01-02 10:00:49.263+: 8349: info : hostname: NSG
2020-01-02 10:00:49.263+: 8349: info : virObjectNew:248 : OBJECT_NEW: 
obj=0x5605a6104de0 classname=virAccessManager
2020-01-02 10:00:49.263+: 8349: info : virObjectNew:248 : OBJECT_NEW: 
obj=0x5605a60f54e0 classname=virAccessManager


my emulator is qemu-3.0 ,i download qemu source code and compiled the emulator:
bash-4.1# /usr/local/bin/qemu-system-x86_64 --version
QEMU emulator version 3.0.0
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers


thanks !







hvm






















destroy
restart
restart






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














































root-vsys_v77
4
2


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

Re: [libvirt] [tck PATCH 2/2] scripts: qemu: Enable disk encryption test

[Daniel P . Berrangé]

On Mon, Nov 25, 2019 at 01:38:13PM +0100, Erik Skultety wrote:
> At the time the test was added, volume encryption wasn't fully
> supported in libvirt. For disks of type 'raw', this is not the case
> anymore, so let's adjust the test according to that fact and enable it.
> 
> Signed-off-by: Erik Skultety 
> ---
>  scripts/qemu/100-disk-encryption.t | 9 -
>  1 file changed, 4 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 :|

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

Re: [libvirt] [tck PATCH 1/2] scripts: qemu: Fix the disk encryption test description

[Daniel P . Berrangé]

On Mon, Nov 25, 2019 at 01:38:12PM +0100, Erik Skultety wrote:
> The original text was a copy-paste from a different test case.
> 
> Signed-off-by: Erik Skultety 
> ---
>  scripts/qemu/100-disk-encryption.t | 7 +++
>  1 file changed, 3 insertions(+), 4 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 :|

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

Re: [libvirt] [tck PATCH 0/2] Enable the storage encryption test

[Erik Skultety]

On Mon, Nov 25, 2019 at 01:38:11PM +0100, Erik Skultety wrote:
> The test was present, but at the time the test was added libvirt (apparently)
> didn't fully support LUKS encryption with disks.
>
> Erik Skultety (2):
>   scripts: qemu: Fix the disk encryption test description
>   scripts: qemu: Enable disk encryption test
>
>  scripts/qemu/100-disk-encryption.t | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)

ping?

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

[libvirt] [PATCH] maint: update to latest gnulib

[Ján Tomko]

Update to:

commit 7d069378921bfa0d7c7198ea177aac0a2440016f
Author: Pádraig Brady 
CommitDate: 2020-01-01 22:00:28 +

   md5, sha1, sha256, sha512: support --with-openssl=auto-gpl-compat

Signed-off-by: Ján Tomko 
---
 .gnulib   | 2 +-
 bootstrap | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Pushed.

diff --git a/.gnulib b/.gnulib
index 1f6fb368c0..7d06937892 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 1f6fb368c04919243e2c70f2aa514a5f88e95309
+Subproject commit 7d069378921bfa0d7c7198ea177aac0a2440016f
diff --git a/bootstrap b/bootstrap
index 5b08e7e2d4..70fd73cc74 100755
--- a/bootstrap
+++ b/bootstrap
@@ -4,7 +4,7 @@ scriptversion=2019-01-04.17; # UTC
 
 # Bootstrap this package from checked-out sources.
 
-# Copyright (C) 2003-2019 Free Software Foundation, Inc.
+# Copyright (C) 2003-2020 Free Software Foundation, Inc.
 
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -166,7 +166,7 @@ bootstrap_epilogue() { :; }
 # specified directory.  Fill in the first %s with the destination
 # directory and the second with the domain name.
 po_download_command_format=\
-"wget --mirror --level=1 -nd -q -A.po -P '%s' \
+"wget --mirror --level=1 -nd -nv -A.po -P '%s' \
  https://translationproject.org/latest/%s/;
 
 # Prefer a non-empty tarname (4th argument of AC_INIT if given), else
-- 
2.19.2

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