[libvirt] [PATCH] gnulib: Pull in latest changes

[Andrea Bolognani]

In particular, we're interested in the following commits:

  commit 43b5194d5b156f8dd7ae576952568d331978f5f0
  Author: Bruno Haible 
  Date:   Sun Jan 5 20:42:12 2020 +0100

tests: Avoid GCC over-optimization caused by _GL_ARG_NONNULL attributes.

* lib/stdlib.in.h: Tweak last commit.

  commit b7d7afe10ddf599452bd80b8a840c830cd474b09
  Author: Bruno Haible 
  Date:   Sun Jan 5 09:13:25 2020 +0100

tests: Avoid GCC over-optimization caused by _GL_ARG_NONNULL attributes.

Reported by Jim Meyering in
.

* lib/stdlib.in.h (GNULIB_defined_canonicalize_file_name): New macro.
(GNULIB_defined_ptsname_r): New macro.
* tests/test-canonicalize.c (_GL_ARG_NONNULL): Define to empty.
(main): Disable the NULL argument test if canonicalize_file_name does
not come from gnulib.
* tests/test-canonicalize-lgpl.c (_GL_ARG_NONNULL): Define to empty.
(main): Disable the NULL argument test if canonicalize_file_name does
not come from gnulib.
* tests/test-ptsname_r.c (_GL_ARG_NONNULL): Define to empty.
(test_errors): Disable the NULL argument test if ptsname_r does not come
from gnulib.

since they fix a build failure caused by the gnulib tests failing
on ppc64le, as reported in

  https://www.redhat.com/archives/libvir-list/2020-January/msg00616.html

Reported-by: Satheesh Rajendran 
Tracked-down-by: Bruno Haible 
Signed-off-by: Andrea Bolognani 
---
Note that I have not actually had the time to confirm this fixes
the problem, or to reproduce it in the first place (I'll do that
tomorrow). Either way, it's way too late for this to make it
into v6.0.0; I'm mostly posting this so that the reporter can
have a go at testing it in the meantime.

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

diff --git a/.gnulib b/.gnulib
index 7d06937892..611869be9f 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 7d069378921bfa0d7c7198ea177aac0a2440016f
+Subproject commit 611869be9f1083e53305446d90a2909fc89914ef
-- 
2.24.1

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

Re: [libvirt] [PATCH 10/12] src: add check for termios.h and conditionally include it

[Pavel Hrdina]

On Tue, Jan 14, 2020 at 03:21:31PM +, Daniel P. Berrangé wrote:
> On Tue, Jan 14, 2020 at 04:12:15PM +0100, Pavel Hrdina wrote:
> > On Fri, Jan 10, 2020 at 03:41:14PM +, Daniel P. Berrangé wrote:
> > > The GNULIB termios module ensures termios.h exists, but
> > > this is not neccessary and libvirt doesn't use any of its
> > > functionality on platforms where it is missing. It is
> > > thus sufficient to conditonallyinclude termios.h
> > > 
> > > Signed-off-by: Daniel P. Berrangé 
> > > ---
> > >  configure.ac   | 1 +
> > >  src/util/virfdstream.c | 4 +++-
> > >  src/util/virfile.c | 4 +++-
> > >  src/util/virutil.c | 1 -
> > >  tools/virsh.h  | 1 -
> > >  tools/vsh.h| 4 +++-
> > >  6 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > This makes the code guarded by HAVE_CFMAKERAW or by !WIN32 and the
> > header by HAVE_TERMIOS_H which makes things more confusing.
> > Should we take the opportunity to unify it?
> 
> Yeah we have alot of this kind of thing, where we could eliminate a
> whole bunch of HAVE_, in favour of just using #ifdef WIN32.
> 
> vircommand.c is a particular case in point where essentially the
> whole file should be ifdef WIN32.
> 
> Taking this kind of approach would eliminate the need to check
> some things in configure.ac and thus cut down what we need for
> meson too.
> 
> Essentially if, given our supported platform list, we can assume
> existance of a header / function on every UNIX platform we care
> about, then I think it is reasonable to use ifdef WIN32 for the
> exception on Windows.
> 
> IOW we only need HAVE_XXX checks if there is variance between UNIX
> platforms we need to identify.

That would be amazing to address all of this some day.  Anyway the
patch is correct regardless of this changes so:

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 11/12] src: replace gmtime_r/localtime_r/strftime with GDateTime

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:15PM +, Daniel P. Berrangé wrote:
> gmtime_r/localtime_r are mostly used in combination with
> strftime to format timestamps in libvirt. This can all
> be replaced with GDateTime resulting in simpler code
> that is also more portable.
> 
> There is some boundary condition problem in parsing POSIX
> timezone offsets in GLib which tickles our test suite.
> The test suite is hacked to avoid the problem. The upsteam
> GLib bug report is

Missing link to the bug report, it's only in the comment in
virtimetest.c.

> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/conf/domain_conf.c   | 11 
>  src/libxl/libxl_domain.c | 10 +++
>  src/qemu/qemu_command.c  | 17 ---
>  src/qemu/qemu_driver.c   | 10 +++
>  src/util/virtime.c   | 35 ---
>  tests/qemuxml2argvmock.c | 12 
>  tests/virtimetest.c  | 39 +
>  tools/virsh-checkpoint.c | 20 +
>  tools/virsh-domain-monitor.c | 14 -
>  tools/virsh-domain.c | 13 -
>  tools/virsh-network.c| 11 
>  tools/virsh-snapshot.c   | 19 -
>  tools/virt-admin.c   | 55 
>  tools/vsh.c  | 16 ---
>  14 files changed, 96 insertions(+), 186 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 10/12] src: add check for termios.h and conditionally include it

[Daniel P . Berrangé]

On Tue, Jan 14, 2020 at 04:12:15PM +0100, Pavel Hrdina wrote:
> On Fri, Jan 10, 2020 at 03:41:14PM +, Daniel P. Berrangé wrote:
> > The GNULIB termios module ensures termios.h exists, but
> > this is not neccessary and libvirt doesn't use any of its
> > functionality on platforms where it is missing. It is
> > thus sufficient to conditonallyinclude termios.h
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> >  configure.ac   | 1 +
> >  src/util/virfdstream.c | 4 +++-
> >  src/util/virfile.c | 4 +++-
> >  src/util/virutil.c | 1 -
> >  tools/virsh.h  | 1 -
> >  tools/vsh.h| 4 +++-
> >  6 files changed, 10 insertions(+), 5 deletions(-)
> 
> This makes the code guarded by HAVE_CFMAKERAW or by !WIN32 and the
> header by HAVE_TERMIOS_H which makes things more confusing.
> Should we take the opportunity to unify it?

Yeah we have alot of this kind of thing, where we could eliminate a
whole bunch of HAVE_, in favour of just using #ifdef WIN32.

vircommand.c is a particular case in point where essentially the
whole file should be ifdef WIN32.

Taking this kind of approach would eliminate the need to check
some things in configure.ac and thus cut down what we need for
meson too.

Essentially if, given our supported platform list, we can assume
existance of a header / function on every UNIX platform we care
about, then I think it is reasonable to use ifdef WIN32 for the
exception on Windows.

IOW we only need HAVE_XXX checks if there is variance between UNIX
platforms we need to identify.


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

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

Re: [libvirt] [PATCH 10/12] src: add check for termios.h and conditionally include it

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:14PM +, Daniel P. Berrangé wrote:
> The GNULIB termios module ensures termios.h exists, but
> this is not neccessary and libvirt doesn't use any of its
> functionality on platforms where it is missing. It is
> thus sufficient to conditonallyinclude termios.h
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  configure.ac   | 1 +
>  src/util/virfdstream.c | 4 +++-
>  src/util/virfile.c | 4 +++-
>  src/util/virutil.c | 1 -
>  tools/virsh.h  | 1 -
>  tools/vsh.h| 4 +++-
>  6 files changed, 10 insertions(+), 5 deletions(-)

This makes the code guarded by HAVE_CFMAKERAW or by !WIN32 and the
header by HAVE_TERMIOS_H which makes things more confusing.
Should we take the opportunity to unify it?

Pavel


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

Re: [libvirt] [PATCH 09/12] src: replace verify(expr) with G_STATIC_ASSERT(expr)

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:13PM +, Daniel P. Berrangé wrote:
> G_STATIC_ASSERT() is a drop-in functional equivalent of
> the GNULIB verify() macro.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  build-aux/syntax-check.mk |  6 --
>  src/conf/snapshot_conf.h  |  2 +-
>  src/conf/virdomaincheckpointobjlist.c |  8 
>  src/esx/esx_network_driver.c  |  2 +-
>  src/esx/esx_storage_backend_iscsi.c   |  2 +-
>  src/esx/esx_storage_backend_vmfs.c|  2 +-
>  src/internal.h|  1 -
>  src/libxl/xen_xm.c|  3 +--
>  src/nwfilter/nwfilter_dhcpsnoop.c |  4 ++--
>  src/qemu/qemu_blockjob.h  |  4 ++--
>  src/qemu/qemu_capabilities.c  |  2 +-
>  src/qemu/qemu_driver.c|  2 +-
>  src/qemu/qemu_firmware.h  |  2 +-
>  src/qemu/qemu_migration_params.c  |  2 +-
>  src/remote/remote_daemon_dispatch.c   | 10 +-
>  src/util/virarch.c|  3 +--
>  src/util/vircgroup.h  |  2 +-
>  src/util/vircrypto.c  |  2 +-
>  src/util/virenum.h|  8 
>  src/util/virinitctl.c |  4 ++--
>  src/util/virkeycode.c | 22 +++---
>  src/util/virmacaddr.h |  2 +-
>  src/util/virobject.h  |  8 
>  src/util/virperf.c|  2 +-
>  src/util/virstoragefile.c |  4 ++--
>  src/util/virtypedparam.h  |  2 +-
>  src/util/virutil.c|  3 +--
>  src/vz/vz_driver.c|  2 +-
>  tests/virstringtest.c |  7 +++
>  tools/virsh-domain.c  |  2 +-
>  tools/virsh-network.c |  2 +-
>  tools/virsh-nodedev.c |  2 +-
>  tools/virsh-pool.c|  2 +-
>  tools/virsh-secret.c  |  2 +-
>  tools/virt-host-validate-common.c |  4 ++--
>  35 files changed, 63 insertions(+), 74 deletions(-)

I would also rename verify to G_STATIC_ASSERT in
examples/c/misc/event-test.c to match G_N_ELEMENTS, but this can
be done as a followup.

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 08/12] util: replace atomic ops impls with g_atomic_int*

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:12PM +, Daniel P. Berrangé wrote:
> Libvirt's original atomic ops impls were largely copied
> from GLib's code at the time. The only API difference
> was that libvirt's virAtomicIntInc() would return a
> value, but g_atomic_int_inc was void. We thus use
> g_atomic_int_add(v, 1) instead, though this means
> virAtomicIntInc() now returns the original value,
> instead of the new value.
> 
> This rewrites libvirt's impl in terms of g_atomic_int*
> as a short term conversion. The key motivation was to
> quickly eliminate use of GNULIB's verify_expr() macro
> which is not a direct match for G_STATIC_ASSERT_EXPR.
> Long term all the callers should be updated to use
> g_atomic_int* directly.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/libxl/libxl_domain.c  |   2 +-
>  src/libxl/libxl_driver.c  |   2 +-
>  src/lxc/lxc_process.c |   4 +-
>  src/nwfilter/nwfilter_dhcpsnoop.c |   6 +-
>  src/qemu/qemu_process.c   |   4 +-
>  src/util/viratomic.h  | 351 ++
>  src/util/virprocess.c |   2 +-
>  tests/viratomictest.c |   2 +-
>  8 files changed, 26 insertions(+), 347 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 3/3] qemu: backup: Implement support for backup disk bitmap name configuration

[Eric Blake]

On 1/9/20 12:31 PM, Peter Krempa wrote:

Use the user-configured name of the bitmap when merging the appropriate
bitmaps for an incremental backup so that the user can see it as
configured. Additionally expose the default bitmap name if nothing is
configured.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_backup.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 2cc0e6ab07..23518a5d40 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -322,7 +322,10 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
  return -1;

  if (incremental) {
-dd->incrementalBitmap = g_strdup_printf("backup-%s", dd->domdisk->dst);
+if (dd->backupdisk->exportbitmap)
+dd->incrementalBitmap = g_strdup(dd->backupdisk->exportbitmap);


Do we need to worry about the user requesting a name that would cause 
conflicts with existing bitmaps in the qcow2 file?  I'm worried this can 
open the door for odd failures if the user accidentally stomps on names 
that libvirt thought were available for its own use.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 2/3] qemu: backup: Implement support for backup disk export name configuration

[Eric Blake]

On 1/9/20 12:31 PM, Peter Krempa wrote:

Pass the exportname as configured when exporting the image via NBD and
fill it with the default if it's not configured.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_backup.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)


Reviewed-by: Eric Blake 



diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index c47de2f4a8..2cc0e6ab07 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -548,9 +548,12 @@ qemuBackupBeginPullExportDisks(virDomainObjPtr vm,
  for (i = 0; i < ndisks; i++) {
  struct qemuBackupDiskData *dd = disks + i;

+if (!dd->backupdisk->exportname)
+dd->backupdisk->exportname = g_strdup(dd->domdisk->dst);
+
  if (qemuMonitorNBDServerAdd(priv->mon,
  dd->store->nodeformat,
-dd->domdisk->dst,
+dd->backupdisk->exportname,
  false,
  dd->incrementalBitmap) < 0)
  return -1;



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 1/3] conf: backup: Allow configuration of names exported via NBD

[Eric Blake]

On 1/9/20 12:31 PM, Peter Krempa wrote:

If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.

Signed-off-by: Peter Krempa 
---
  docs/formatbackup.html.in |  9 +
  docs/schemas/domainbackup.rng |  8 
  src/conf/backup_conf.c| 11 +++
  src/conf/backup_conf.h|  2 ++
  tests/domainbackupxml2xmlin/backup-pull-seclabel.xml  |  2 +-
  tests/domainbackupxml2xmlout/backup-pull-seclabel.xml |  2 +-
  6 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 1c690901c7..7d2c6f1599 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -85,6 +85,15 @@
Setting this attribute to yes(default) 
specifies
  that the disk should take part in the backup and using
  no excludes the disk from the backup.
+  exportname
+  Allows to modify the NBD export name for the given disk.


Allows modification of the NBD export name


+By default equal to disk target.
+Valid only for pull mode backups. 


Is the space after '.' needed?


+  exportbitmap
+  Allows to modify the name of the bitmap describing dirty


Allows modification of the name


+blocks for an incremental backup exported via NBD export name
+for the given disk.
+Valid only for pull mode backups. 


Another trailing space

Otherwise looks fine, once you also fix Daniel's nit.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

[libvirt] [PATCH 8/8] qemu-slirp: register helper for migration

[marcandre . lureau]

From: Marc-André Lureau 

When the helper supports DBus, connect it to the bus and set its ID.

If the helper supports migration, register its ID to the list of
dbus-vmstate ID to migrate, and specify --dbus-incoming when
restoring the VM.

Signed-off-by: Marc-André Lureau 
---
 src/qemu/qemu_slirp.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
index 8e001f0d10..48fc0a68c2 100644
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -18,6 +18,7 @@
 
 #include 
 
+#include "qemu_dbus.h"
 #include "qemu_extdevice.h"
 #include "qemu_security.h"
 #include "qemu_slirp.h"
@@ -202,6 +203,16 @@ qemuSlirpGetFD(qemuSlirpPtr slirp)
 }
 
 
+static char *
+qemuSlirpGetDBusVMStateId(virDomainNetDefPtr net)
+{
+char macstr[VIR_MAC_STRING_BUFLEN] = "";
+
+/* can't use alias, because it's not stable across restarts */
+return g_strdup_printf("slirp-%s", virMacAddrFormat(>mac, macstr));
+}
+
+
 void
 qemuSlirpStop(qemuSlirpPtr slirp,
   virDomainObjPtr vm,
@@ -209,11 +220,14 @@ qemuSlirpStop(qemuSlirpPtr slirp,
   virDomainNetDefPtr net)
 {
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *id = qemuSlirpGetDBusVMStateId(net);
 g_autofree char *pidfile = NULL;
 virErrorPtr orig_err;
 pid_t pid;
 int rc;
 
+qemuDBusVMStateRemove(vm, id);
+
 if (!(pidfile = qemuSlirpCreatePidFilename(cfg, vm->def, 
net->info.alias))) {
 VIR_WARN("Unable to construct slirp pidfile path");
 return;
@@ -310,6 +324,29 @@ qemuSlirpStart(qemuSlirpPtr slirp,
 }
 }
 
+if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_DBUS_ADDRESS)) {
+g_autofree char *id = qemuSlirpGetDBusVMStateId(net);
+g_autofree char *dbus_addr = qemuDBusGetAddress(driver, vm);
+
+if (qemuDBusStart(driver, vm) < 0)
+return -1;
+
+virCommandAddArgFormat(cmd, "--dbus-id=%s", id);
+
+virCommandAddArgFormat(cmd, "--dbus-address=%s", dbus_addr);
+
+if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
+if (qemuDBusVMStateAdd(vm, id) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to register slirp migration"));
+return -1;
+}
+if (incoming) {
+virCommandAddArg(cmd, "--dbus-incoming");
+}
+}
+}
+
 if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT))
 virCommandAddArg(cmd, "--exit-with-parent");
 
-- 
2.25.0.rc2.1.g09a9a1a997

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

[libvirt] [PATCH 7/8] qemu: add dbus-vmstate helper migration support

[marcandre . lureau]

From: Marc-André Lureau 

Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".

libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.

See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst

Signed-off-by: Marc-André Lureau 
---
 src/qemu/qemu_alias.c|  7 +++
 src/qemu/qemu_alias.h|  2 +
 src/qemu/qemu_command.c  | 62 +++
 src/qemu/qemu_command.h  |  3 ++
 src/qemu/qemu_dbus.c | 14 ++
 src/qemu/qemu_dbus.h |  4 ++
 src/qemu/qemu_domain.c   | 10 +
 src/qemu/qemu_domain.h   |  5 +++
 src/qemu/qemu_hotplug.c  | 82 
 src/qemu/qemu_hotplug.h  |  8 
 src/qemu/qemu_migration.c| 51 ++
 src/qemu/qemu_monitor.c  | 21 +
 src/qemu/qemu_monitor.h  |  3 ++
 src/qemu/qemu_monitor_json.c | 15 +++
 src/qemu/qemu_monitor_json.h |  5 +++
 15 files changed, 292 insertions(+)

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 61f8ce05c9..d2e1ce155e 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -840,3 +840,10 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias)
 
 return ret;
 }
+
+
+const char *
+qemuDomainGetDBusVMStateAlias(void)
+{
+return "dbus-vmstate0";
+}
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index aaac09a1d1..e3492116c5 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -95,3 +95,5 @@ char *qemuAliasChardevFromDevAlias(const char *devAlias)
 const char *qemuDomainGetManagedPRAlias(void);
 
 char *qemuDomainGetUnmanagedPRAlias(const char *parentalias);
+
+const char *qemuDomainGetDBusVMStateAlias(void);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7429a0b7f5..53051a8726 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -24,6 +24,7 @@
 #include "qemu_command.h"
 #include "qemu_hostdev.h"
 #include "qemu_capabilities.h"
+#include "qemu_dbus.h"
 #include "qemu_interface.h"
 #include "qemu_alias.h"
 #include "qemu_security.h"
@@ -9458,6 +9459,64 @@ qemuBuildPflashBlockdevCommandLine(virCommandPtr cmd,
 }
 
 
+static virJSONValuePtr
+qemuBuildDBusVMStateInfoPropsInternal(const char *alias,
+  const char *addr)
+{
+virJSONValuePtr ret = NULL;
+
+if (qemuMonitorCreateObjectProps(,
+ "dbus-vmstate", alias,
+ "s:addr", addr, NULL) < 0)
+return NULL;
+
+return ret;
+}
+
+
+virJSONValuePtr
+qemuBuildDBusVMStateInfoProps(virQEMUDriverPtr driver,
+  virDomainObjPtr vm)
+{
+g_autofree char *addr = qemuDBusGetAddress(driver, vm);
+
+return 
qemuBuildDBusVMStateInfoPropsInternal(qemuDomainGetDBusVMStateAlias(),
+ addr);
+}
+
+
+static int
+qemuBuildDBusVMStateCommandLine(virCommandPtr cmd,
+virQEMUDriverPtr driver,
+virDomainObjPtr vm)
+{
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+g_autoptr(virJSONValue) props = NULL;
+qemuDomainObjPrivatePtr priv = QEMU_DOMAIN_PRIVATE(vm);
+
+if (virStringListLength((const char **)priv->dbusVMStateIds) == 0)
+return 0;
+
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
+VIR_INFO("dbus-vmstate object is not supported by this QEMU binary");
+return 0;
+}
+
+if (!(props = qemuBuildDBusVMStateInfoProps(driver, vm)))
+return -1;
+
+if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
+return -1;
+
+virCommandAddArg(cmd, "-object");
+virCommandAddArgBuffer(cmd, );
+
+priv->dbusVMState = true;
+
+return 0;
+}
+
+
 /**
  * qemuBuildCommandLineValidate:
  *
@@ -9689,6 +9748,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0)
 return NULL;
 
+if (qemuBuildDBusVMStateCommandLine(cmd, driver, vm) < 0)
+return NULL;
+
 if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0)
 return NULL;
 
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index cb8c394fb6..239d3daede 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -59,6 +59,9 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver,
 virJSONValuePtr qemuBuildPRManagerInfoProps(virStorageSourcePtr src);
 virJSONValuePtr qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivatePtr 
priv);
 
+virJSONValuePtr qemuBuildDBusVMStateInfoProps(virQEMUDriverPtr driver,
+  virDomainObjPtr vm);
+
 /* Generate the object properties for a secret */
 int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
  

[libvirt] [PATCH 6/8] qemu: prepare and stop the dbus daemon

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 src/qemu/qemu_process.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4195042194..845a7caa55 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -58,6 +58,7 @@
 #include "qemu_extdevice.h"
 #include "qemu_firmware.h"
 #include "qemu_backup.h"
+#include "qemu_dbus.h"
 
 #include "cpu/cpu.h"
 #include "cpu/cpu_x86.h"
@@ -6457,6 +6458,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
+if (qemuDBusPrepareHost(driver) < 0)
+return -1;
+
 if (qemuPrepareNVRAM(cfg, vm) < 0)
 return -1;
 
@@ -7378,6 +7382,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
 qemuExtDevicesStop(driver, vm);
 
+qemuDBusStop(driver, vm);
+
 vm->def->id = -1;
 
 /* Stop autodestroy in case guest is restarted */
-- 
2.25.0.rc2.1.g09a9a1a997

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

[libvirt] [PATCH 1/8] qemu: remove dbus-vmstate code

[marcandre . lureau]

From: Marc-André Lureau 

This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.

Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.

Signed-off-by: Marc-André Lureau 
---
 src/qemu/Makefile.inc.am  |   2 -
 src/qemu/qemu_alias.c |  16 -
 src/qemu/qemu_alias.h |   3 -
 src/qemu/qemu_command.c   |  83 -
 src/qemu/qemu_command.h   |   3 -
 src/qemu/qemu_dbus.c  |  94 
 src/qemu/qemu_dbus.h  |  42 -
 src/qemu/qemu_domain.c|  13 
 src/qemu/qemu_domain.h|   1 -
 src/qemu/qemu_extdevice.c |   4 +-
 src/qemu/qemu_hotplug.c   |  83 +
 src/qemu/qemu_hotplug.h   |  11 
 src/qemu/qemu_migration.c |   8 ---
 src/qemu/qemu_slirp.c | 125 +-
 src/qemu/qemu_slirp.h |   4 +-
 15 files changed, 7 insertions(+), 485 deletions(-)
 delete mode 100644 src/qemu/qemu_dbus.c
 delete mode 100644 src/qemu/qemu_dbus.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 967f6e75a2..028ab9043c 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -13,8 +13,6 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_capabilities.h \
qemu/qemu_command.c \
qemu/qemu_command.h \
-   qemu/qemu_dbus.c \
-   qemu/qemu_dbus.h \
qemu/qemu_domain.c \
qemu/qemu_domain.h \
qemu/qemu_domain_address.c \
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 93bdcb7548..61f8ce05c9 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -840,19 +840,3 @@ qemuDomainGetUnmanagedPRAlias(const char *parentalias)
 
 return ret;
 }
-
-char *
-qemuAliasDBusVMStateFromId(const char *id)
-{
-char *ret;
-size_t i;
-
-ret = g_strdup_printf("dbus-vms-%s", id);
-
-for (i = 0; ret[i]; i++) {
-if (ret[i] == ':')
-ret[i] = '_';
-}
-
-return ret;
-}
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index ae2fce16bc..aaac09a1d1 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -95,6 +95,3 @@ char *qemuAliasChardevFromDevAlias(const char *devAlias)
 const char *qemuDomainGetManagedPRAlias(void);
 
 char *qemuDomainGetUnmanagedPRAlias(const char *parentalias);
-
-char *qemuAliasDBusVMStateFromId(const char *id)
-ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 904d2beab5..7429a0b7f5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -27,7 +27,6 @@
 #include "qemu_interface.h"
 #include "qemu_alias.h"
 #include "qemu_security.h"
-#include "qemu_dbus.h"
 #include "qemu_slirp.h"
 #include "qemu_block.h"
 #include "cpu/cpu.h"
@@ -9459,85 +9458,6 @@ qemuBuildPflashBlockdevCommandLine(virCommandPtr cmd,
 }
 
 
-static virJSONValuePtr
-qemuBuildDBusVMStateInfoPropsInternal(const char *alias,
-  const char *addr)
-{
-virJSONValuePtr ret = NULL;
-
-if (qemuMonitorCreateObjectProps(,
- "dbus-vmstate", alias,
- "s:addr", addr, NULL) < 0)
-return NULL;
-
-return ret;
-}
-
-
-virJSONValuePtr
-qemuBuildDBusVMStateInfoProps(const char *id,
-  const char *addr)
-{
-g_autofree char *alias = qemuAliasDBusVMStateFromId(id);
-
-if (!alias)
-return NULL;
-
-return qemuBuildDBusVMStateInfoPropsInternal(alias, addr);
-}
-
-
-typedef struct qemuBuildDBusVMStateCommandLineData {
-virCommandPtr cmd;
-} qemuBuildDBusVMStateCommandLineData;
-
-
-static int
-qemuBuildDBusVMStateCommandLineEach(void *payload,
-const void *id,
-void *user_data)
-{
-qemuBuildDBusVMStateCommandLineData *data = user_data;
-qemuDBusVMStatePtr vms = payload;
-g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-g_autoptr(virJSONValue) props = NULL;
-
-if (!(props = qemuBuildDBusVMStateInfoProps(id, vms->addr)))
-return -1;
-
-if (virQEMUBuildObjectCommandlineFromJSON(, props) < 0)
-return -1;
-
-virCommandAddArg(data->cmd, "-object");
-virCommandAddArgBuffer(data->cmd, );
-
-return 0;
-}
-
-static int
-qemuBuildDBusVMStateCommandLine(virCommandPtr cmd,
-qemuDomainObjPrivatePtr priv)
-{
-qemuBuildDBusVMStateCommandLineData data = {
-.cmd = cmd,
-};
-
-if (virHashSize(priv->dbusVMStates) == 0)
-return 0;
-
-if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("dbus-vmstate object is not supported by this QEMU 
binary"));
-return 0;
-}
-
-if (virHashForEach(priv->dbusVMStates, 

[libvirt] [PATCH 5/8] domain: save/restore the state of dbus-daemon running

[marcandre . lureau]

From: Marc-André Lureau 

This avoids trying to start a dbus-daemon when its already running.

Signed-off-by: Marc-André Lureau 
---
 src/qemu/qemu_domain.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7722a53c62..dda3cb781f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
   virDomainChrTypeToString(priv->monConfig->type));
 }
 
+if (priv->dbusDaemonRunning)
+virBufferAddLit(buf, "\n");
+
 if (priv->namespaces) {
 ssize_t ns = -1;
 
@@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 goto error;
 }
 
+priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 
0;
+
 if ((node = virXPathNode("./namespaces", ctxt))) {
 xmlNodePtr next;
 
-- 
2.25.0.rc2.1.g09a9a1a997

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

[libvirt] [PATCH 4/8] qemu: add a DBus daemon helper unit

[marcandre . lureau]

From: Marc-André Lureau 

Add a unit to start & stop a private dbus-daemon.

The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.

The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst

Signed-off-by: Marc-André Lureau 
---
 src/qemu/Makefile.inc.am |   4 +
 src/qemu/qemu_dbus.c | 299 +++
 src/qemu/qemu_dbus.h |  40 ++
 src/qemu/qemu_domain.c   |   2 +
 src/qemu/qemu_domain.h   |   3 +
 tests/Makefile.am|   1 +
 6 files changed, 349 insertions(+)
 create mode 100644 src/qemu/qemu_dbus.c
 create mode 100644 src/qemu/qemu_dbus.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 028ab9043c..833638ec3c 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -69,6 +69,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_checkpoint.h \
qemu/qemu_backup.c \
qemu/qemu_backup.h \
+   qemu/qemu_dbus.c \
+   qemu/qemu_dbus.h \
$(NULL)
 
 
@@ -93,6 +95,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
$(LIBNL_CFLAGS) \
$(SELINUX_CFLAGS) \
$(XDR_CFLAGS) \
+   $(DBUS_CFLAGS) \
-I$(srcdir)/access \
-I$(builddir)/access \
-I$(srcdir)/conf \
@@ -105,6 +108,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(SELINUX_LIBS) \
+   $(DBUS_LIBS) \
$(LIBXML_LIBS) \
$(NULL)
 libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
new file mode 100644
index 00..9c8a03c947
--- /dev/null
+++ b/src/qemu/qemu_dbus.c
@@ -0,0 +1,299 @@
+/*
+ * qemu_dbus.c: QEMU dbus daemon
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "qemu_extdevice.h"
+#include "qemu_dbus.h"
+#include "qemu_security.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("qemu.dbus");
+
+
+int
+qemuDBusPrepareHost(virQEMUDriverPtr driver)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+VIR_DIR_CREATE_ALLOW_EXIST);
+}
+
+
+static char *
+qemuDBusCreatePidFilename(const char *stateDir,
+  const char *shortName)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virPidFileBuildPath(stateDir, name);
+}
+
+
+static char *
+qemuDBusCreateFilename(const char *stateDir,
+   const char *shortName,
+   const char *ext)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virFileBuildPath(stateDir, name,  ext);
+}
+
+
+static char *
+qemuDBusCreateSocketPath(virQEMUDriverConfigPtr cfg,
+ const char *shortName)
+{
+return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".sock");
+}
+
+
+char *
+qemuDBusGetAddress(virQEMUDriverPtr driver,
+   virDomainObjPtr vm)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *shortName = virDomainDefGetShortName(vm->def);
+g_autofree char *path = qemuDBusCreateSocketPath(cfg, shortName);
+
+return g_strdup_printf("unix:path=%s", path);
+}
+
+
+static int
+qemuDBusGetPid(const char *binPath,
+   const char *stateDir,
+   const char *shortName,
+   pid_t *pid)
+{
+g_autofree char *pidfile = qemuDBusCreatePidFilename(stateDir, shortName);
+
+return virPidFileReadPathIfAlive(pidfile, pid, binPath);
+}
+
+
+static int
+qemuDBusWriteConfig(const char *filename, const char *path)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+g_autofree char *config = NULL;
+
+virBufferAddLit(, "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd\;>\n");
+virBufferAddLit(, "\n");
+virBufferAdjustIndent(, 2);
+
+virBufferAddLit(, "org.libvirt.qemu\n");
+virBufferAsprintf(, "unix:path=%s\n", path);
+virBufferAddLit(, "EXTERNAL\n");
+
+virBufferAddLit(, 

[libvirt] [PATCH 2/8] qemu-conf: add configurable dbus-daemon location

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 m4/virt-driver-qemu.m4 | 6 ++
 src/qemu/libvirtd_qemu.aug | 1 +
 src/qemu/qemu.conf | 3 +++
 src/qemu/qemu_conf.c   | 5 +
 src/qemu/qemu_conf.h   | 1 +
 src/qemu/test_libvirtd_qemu.aug.in | 1 +
 6 files changed, 17 insertions(+)

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index a1d2c66bba..886261fce5 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -110,6 +110,12 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
[/usr/bin:/usr/libexec])
   AC_DEFINE_UNQUOTED([QEMU_SLIRP_HELPER], ["$QEMU_SLIRP_HELPER"],
  [QEMU slirp helper])
+
+  AC_PATH_PROG([QEMU_DBUS_DAEMON], [dbus-daemon],
+   [/usr/bin/dbus-daemon],
+   [/usr/bin:/usr/libexec])
+  AC_DEFINE_UNQUOTED([QEMU_DBUS_DAEMON], ["$QEMU_DBUS_DAEMON"],
+ [QEMU dbus daemon])
 ])
 
 AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 557b6f38f8..58a2b6416f 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -89,6 +89,7 @@ module Libvirtd_qemu =
  | str_entry "bridge_helper"
  | str_entry "pr_helper"
  | str_entry "slirp_helper"
+ | str_entry "dbus_daemon"
  | bool_entry "set_process_name"
  | int_entry "max_processes"
  | int_entry "max_files"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index b6805ffc41..72ad4d4a10 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -828,6 +828,9 @@
 # Path to the SLIRP networking helper.
 #slirp_helper = "/usr/bin/slirp-helper"
 
+# Path to the dbus-daemon
+#dbus_daemon = "/usr/bin/dbus-daemon"
+
 # User for the swtpm TPM Emulator
 #
 # Default is 'tss'; this is the same user that tcsd (TrouSerS) installs
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b62dd1df52..e1fea390c7 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -228,6 +228,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 cfg->bridgeHelperName = g_strdup(QEMU_BRIDGE_HELPER);
 cfg->prHelperName = g_strdup(QEMU_PR_HELPER);
 cfg->slirpHelperName = g_strdup(QEMU_SLIRP_HELPER);
+cfg->slirpHelperName = g_strdup(QEMU_DBUS_DAEMON);
 
 cfg->securityDefaultConfined = true;
 cfg->securityRequireConfined = false;
@@ -313,6 +314,7 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->bridgeHelperName);
 VIR_FREE(cfg->prHelperName);
 VIR_FREE(cfg->slirpHelperName);
+VIR_FREE(cfg->dbusDaemonName);
 
 VIR_FREE(cfg->saveImageFormat);
 VIR_FREE(cfg->dumpImageFormat);
@@ -604,6 +606,9 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr 
cfg,
 if (virConfGetValueString(conf, "slirp_helper", >slirpHelperName) < 0)
 return -1;
 
+if (virConfGetValueString(conf, "dbus_daemon", >dbusDaemonName) < 0)
+return -1;
+
 if (virConfGetValueBool(conf, "set_process_name", >setProcessName) < 
0)
 return -1;
 if (virConfGetValueUInt(conf, "max_processes", >maxProcesses) < 0)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index b9401635d7..6354925e0b 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -155,6 +155,7 @@ struct _virQEMUDriverConfig {
 char *bridgeHelperName;
 char *prHelperName;
 char *slirpHelperName;
+char *dbusDaemonName;
 
 bool macFilter;
 
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index dd90edf687..95540a4b5e 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -104,6 +104,7 @@ module Test_libvirtd_qemu =
 { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
 { "pr_helper" = "/usr/bin/qemu-pr-helper" }
 { "slirp_helper" = "/usr/bin/slirp-helper" }
+{ "dbus_daemon" = "/usr/bin/dbus-daemon" }
 { "swtpm_user" = "tss" }
 { "swtpm_group" = "tss" }
 { "capability_filters"
-- 
2.25.0.rc2.1.g09a9a1a997

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

[libvirt] [PATCH 3/8] qemu-conf: add dbusStateDir

[marcandre . lureau]

From: Marc-André Lureau 

Location of DBus daemon state configuration, socket, pid...

Signed-off-by: Marc-André Lureau 
---
 src/qemu/qemu_conf.c | 4 
 src/qemu/qemu_conf.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e1fea390c7..ade12dac6c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -144,6 +144,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 
 cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu", 
LOCALSTATEDIR);
 
+cfg->dbusStateDir = g_strdup_printf("%s/run/libvirt/qemu/dbus", 
LOCALSTATEDIR);
+
 cfg->libDir = g_strdup_printf("%s/lib/libvirt/qemu", LOCALSTATEDIR);
 cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir);
 cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir);
@@ -174,6 +176,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
 
 cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);
+cfg->dbusStateDir = g_strdup_printf("%s/dbus", cfg->stateDir);
 
 cfg->configBaseDir = virGetUserConfigDirectory();
 
@@ -274,6 +277,7 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->stateDir);
 VIR_FREE(cfg->swtpmStateDir);
 VIR_FREE(cfg->slirpStateDir);
+VIR_FREE(cfg->dbusStateDir);
 
 VIR_FREE(cfg->libDir);
 VIR_FREE(cfg->cacheDir);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 6354925e0b..de6430cf97 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -97,6 +97,7 @@ struct _virQEMUDriverConfig {
 char *stateDir;
 char *swtpmStateDir;
 char *slirpStateDir;
+char *dbusStateDir;
 /* These two directories are ones QEMU processes use (so must match
  * the QEMU user/group */
 char *libDir;
-- 
2.25.0.rc2.1.g09a9a1a997

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

[libvirt] [PATCH 0/8] Second take on slirp-helper & dbus-vmstate

[marcandre . lureau]

From: Marc-André Lureau 

Hi,

The series "[libvirt] [PATCH v2 00/23] Use a slirp helper process" has
been merged and partially reverted. Meanwhile, qemu dbus-vmstate
design has been changed and merged upstream.

This new series fixes the slirp-helper support. The significant change
is that dbus-vmstate now requires a bus (instead of the earlier
peer-to-peer connection). The current series doesn't attempt to
enforce strict policies on the bus. As long as you can connect to the
bus, you can send/receive from/to anyone. A follow-up series should
implement the recommendations from
https://qemu.readthedocs.io/en/latest/interop/dbus.html#security.

The libslirp-rs slirp-helper hasn't yet received an official release.
For testing, you may:
$ cargo install --features=all --git 
https://gitlab.freedesktop.org/slirp/libslirp-rs

The resulting binary should be ~/.cargo/bin/slirp-helper, so qemu.conf
slirp_helper location should be adjusted. With that in place, a VM
with user networking (slirp) should now start with the helper process.

thanks

Marc-André Lureau (8):
  qemu: remove dbus-vmstate code
  qemu-conf: add configurable dbus-daemon location
  qemu-conf: add dbusStateDir
  qemu: add a DBus daemon helper unit
  domain: save/restore the state of dbus-daemon running
  qemu: prepare and stop the dbus daemon
  qemu: add dbus-vmstate helper migration support
  qemu-slirp: register helper for migration

 m4/virt-driver-qemu.m4 |   6 +
 src/qemu/Makefile.inc.am   |   6 +-
 src/qemu/libvirtd_qemu.aug |   1 +
 src/qemu/qemu.conf |   3 +
 src/qemu/qemu_alias.c  |  17 +-
 src/qemu/qemu_alias.h  |   3 +-
 src/qemu/qemu_command.c|  65 +++
 src/qemu/qemu_command.h|   6 +-
 src/qemu/qemu_conf.c   |   9 +
 src/qemu/qemu_conf.h   |   2 +
 src/qemu/qemu_dbus.c   | 283 +
 src/qemu/qemu_dbus.h   |  30 +--
 src/qemu/qemu_domain.c |  30 +--
 src/qemu/qemu_domain.h |   9 +-
 src/qemu/qemu_extdevice.c  |   4 +-
 src/qemu/qemu_hotplug.c| 165 +
 src/qemu/qemu_hotplug.h|  17 +-
 src/qemu/qemu_migration.c  |  57 +-
 src/qemu/qemu_monitor.c|  21 +++
 src/qemu/qemu_monitor.h|   3 +
 src/qemu/qemu_monitor_json.c   |  15 ++
 src/qemu/qemu_monitor_json.h   |   5 +
 src/qemu/qemu_process.c|   6 +
 src/qemu/qemu_slirp.c  | 126 ++---
 src/qemu/qemu_slirp.h  |   4 +-
 src/qemu/test_libvirtd_qemu.aug.in |   1 +
 tests/Makefile.am  |   1 +
 27 files changed, 564 insertions(+), 331 deletions(-)

-- 
2.25.0.rc2.1.g09a9a1a997

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

Re: [libvirt] libvirt 6.0.0 make check-TESTS fails "FAIL: test-ptsname_r"

[Satheesh Rajendran]

On Mon, Jan 13, 2020 at 04:13:43PM -0300, Daniel Henrique Barboza wrote:
> Satheesh,
> 
> Did you update the .gnulib submodule as well? I am asking this because commit
> 
Hi Daniel,

1. Thanks for reverting back, I guess libvirt repository yet to update 
submodule 
with those commit, it is still pointing to 
7d069378921bfa0d7c7198ea177aac0a2440016f 
a little older.

2. Even if I update .gnulib submodule to point to origin/master, it gets revert 
back to
old commit while running autogen.sh

# git submodule foreach git status
Entering '.gnulib'
On branch master
Your branch is up to date with 'origin/master'.
...

# ../libvirt/autogen.sh -prefix=/usr/share/avocado-plugins-vt/bin
Updating submodules...
Submodule path '.gnulib': checked out '7d069378921bfa0d7c7198ea177aac0a2440016f'
Running autoreconf...
...

# git submodule foreach git status
Entering '.gnulib'
HEAD detached at 7d0693789
nothing to commit, working tree clean
...

3. Then pointing gnulib explicitly with latest commits with --no-git worked
#  GNULIB_SRCDIR=/home/sath/gnulib  ../libvirt/autogen.sh --no-git
..

But it would be good to have libvirt submodule updated with latest 
gnulib commit to getrid of this issue.

Thanks!

Regards,
-Satheesh.


> 
> commit b7d7afe10ddf599452bd80b8a840c830cd474b09
> Author: Bruno Haible 
> Date:   Sun Jan 5 09:13:25 2020 +0100
> 
> tests: Avoid GCC over-optimization caused by _GL_ARG_NONNULL attributes.
> Reported by Jim Meyering in
> .
> 
> 
> Is putting run restrictions in the test failing for you. In a quick glance, it
> looks like this commit should avoid the problem.
> 
> You can update the .gnulib module by going into the .gnulib dir, then
> 'git checkout master' and 'git pull'.
> 
> 
> Thanks,
> 
> 
> DHB
> 
> 
> 
> On 1/13/20 1:00 PM, Satheesh Rajendran wrote:
> > Hi,
> > 
> > libvirt 6.0.0 (commit:bfd5f69d6038daeac03d2684fcd98aeee4ef1e24), "make rpm" 
> > fails at
> > make check-TESTS
> > 
> > ...
> > ..
> > PASS: test-memchr
> > PASS: test-send
> > ../../../build-aux/test-driver: line 107: 453114 Aborted 
> > (core dumped) "$@" > $log_file 2>&1
> > FAIL: test-ptsname_r
> > PASS: test-setsockopt
> > PASS: test-sigaction
> > PASS: test-signal-h
> > ...
> > ---
> > # cat 
> > /root/rpmbuild/BUILD/libvirt-6.0.0/ppc64le-redhat-linux-gnu/gnulib/tests/test-suite.log
> > 
> > libvirt 6.0.0: gnulib/tests/test-suite.log
> > 
> > 
> > # TOTAL: 149
> > # PASS:  148
> > # SKIP:  0
> > # XFAIL: 0
> > # FAIL:  1
> > # XPASS: 0
> > # ERROR: 0
> > 
> > .. contents:: :depth: 2
> > 
> > FAIL: test-ptsname_r
> > 
> > 
> > ../../../gnulib/tests/test-ptsname_r.c:89: assertion 'result == EINVAL' 
> > failed
> > FAIL test-ptsname_r (exit status: 134)
> > 
> > 
> > 
> > 
> > Env:
> > HW: Power8
> > ArcH: ppc64le
> > OS: Fedora 31
> > GCC: 9.2.1 20190827 (Red Hat 9.2.1-1)
> > Glibc: glibc-2.30-8.fc31.ppc64le
> > 
> > 
> > Gdb output:
> > 
> > # gdb 
> > /root/rpmbuild/BUILD/libvirt-6.0.0/ppc64le-redhat-linux-gnu/gnulib/tests/test-ptsname_r
> >  -c 
> > core.test-ptsname_r.0.1a9e4ce7d7ec4d7e81e295f7f67c7029.317503.1578922893
> > GNU gdb (GDB) Fedora 8.3.50.20190824-26.fc31
> > ..
> > ..
> > Reading symbols from 
> > /root/rpmbuild/BUILD/libvirt-6.0.0/ppc64le-redhat-linux-gnu/gnulib/tests/test-ptsname_r...
> > [New LWP 317503]
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib64/libthread_db.so.1".
> > Core was generated by `./test-ptsname_r '.
> > Program terminated with signal SIGABRT, Aborted.
> > #0  0x7fffac447358 in __libc_signal_restore_set (set=0x7fffefdc3078) at 
> > ../sysdeps/unix/sysv/linux/internal-signals.h:84
> > 84return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, set, 
> > NULL,
> > (gdb) start
> > Temporary breakpoint 1 at 0x11d850c88: file 
> > ../../../gnulib/tests/test-ptsname_r.c, line 94.
> > Starting program: 
> > /root/rpmbuild/BUILD/libvirt-6.0.0/ppc64le-redhat-linux-gnu/gnulib/tests/test-ptsname_r
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib64/libthread_db.so.1".
> > Temporary breakpoint 1, 0x00010c88 in main () at 
> > ../../../gnulib/tests/test-ptsname_r.c:94
> > 94  {
> > (gdb) c
> > Continuing.
> > ../../../gnulib/tests/test-ptsname_r.c:89: assertion 'result == EINVAL' 
> > failed
> > Program received signal SIGABRT, Aborted.
> > 0x77d57358 in __libc_signal_restore_set (set=0x7fffe438) at 
> > ../sysdeps/unix/sysv/linux/internal-signals.h:84
> > 84return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, set, 
> > NULL,
> > (gdb) print result
> > $1 = '\000' 
> > (gdb)
> > 
> > Any help would be appreciated, Thanks in advance.
> > 
> > Regards,
> > -Satheesh.
> > 
> > 
> > --
> > libvir-list mailing list
> > 

Re: [libvirt] [PATCH] news: News for RDT-MBM feature

[Han Han]

On Tue, Jan 14, 2020 at 4:23 PM Andrea Bolognani 
wrote:

> On Tue, 2020-01-14 at 13:48 +0800, Han Han wrote:
> > +  
> > +
> > +  qemu: Support to report the stats of memory bandwidth usage
> > +
> > +
> > +  Implement Intel RDT-MBM to libvirt. The stats can be got via
> > +  virsh domstats --memory.
> > +
> > +  
>
> Oops, sorry for missing this while updating the release notes, and
> thanks for submitting the update yourself!
>
You're welcome :)

>
>   Reviewed-by: Andrea Bolognani 
>
> Tweaked it a bit and pushed.
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
>

-- 
Best regards,
---
Han Han
Quality Engineer
Redhat.

Email: h...@redhat.com
Phone: +861065339333
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 7/7] conf: do not generate machine names ending with a dash

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:41AM +0100, Ján Tomko wrote:
> As of systemd commit:
>
> commit d65652f1f21a4b0c59711320f34266c635393c89
> Author: Zbigniew Jędrzejewski-Szmek 
> CommitDate: 2018-12-10 09:56:56 +0100
>
> Partially unify hostname_is_valid() and dns_name_is_valid()
>
> Dashes are no longer allowed at the end of machine names.
>
> Trim the trailing dashes from the generated name before passing
> it to machined.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1790409
>
> Signed-off-by: Ján Tomko 
> ---
>  src/conf/domain_conf.c | 3 +++
>  tests/virsystemdtest.c | 4 
>  2 files changed, 7 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1290241923..512b7c49d2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30467,6 +30467,9 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
>
>  virBufferAddChar(buf, *name);
>  }
> +
> +/* trailing dashes are not allowed */
> +virBufferTrimChars(buf, "-");
>  }
>
>  #undef HOSTNAME_CHARS
> diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
> index 9b95ca6789..26876850b8 100644
> --- a/tests/virsystemdtest.c
> +++ b/tests/virsystemdtest.c
> @@ -740,6 +740,10 @@ mymain(void)
>   
> "qemu-7-123456789012345678901234567890123456789012345678901234567");
>  
> TEST_MACHINE("123456789012345678901234567890123456789012345678901234567890", 
> 8,
>   
> "qemu-8-123456789012345678901234567890123456789012345678901234567");
> +
> TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)",
>  100,
> + 
> "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec");
> +
> TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)",
>  10,
> + 
> "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec");

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 04/12] util: add detection of openpty function

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:08PM +, Daniel P. Berrangé wrote:
> All UNIX platforms we care about have openpty() in the libutil
> library. Use of pty.h must also be made conditional, excluding
> Win32.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  configure.ac |  4 
>  src/util/Makefile.inc.am |  1 +
>  src/util/virfile.c   | 14 +-
>  3 files changed, 18 insertions(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 03/12] build: validate headers against local gnulib not git repo

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:07PM +, Daniel P. Berrangé wrote:
> Some syntax check rules validate usage of headers provided
> by gnulib. We want to validate these only against the gnulib
> modules we've chosen to use, not all modules, since we're
> trying to eliminate them.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  build-aux/syntax-check.mk | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 02/12] tests: always declare environ

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:06PM +, Daniel P. Berrangé wrote:
> Some UNIX platforms don't declare 'environ' in their
> header files. We can unconditionally declare it ourselves
> to avoid this problem.
> 
> There is no need to do this in the aa-helper code
> since that is Linux only code.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  tests/commandhelper.c | 3 +++
>  tests/commandtest.c   | 3 +++
>  2 files changed, 6 insertions(+)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 01/12] src: import gnulib's intprops.h header

[Pavel Hrdina]

On Fri, Jan 10, 2020 at 03:41:05PM +, Daniel P. Berrangé wrote:
> This copies intprops.h to virintprops.h. A couple of
> conditionals were cut out since we don't need to support
> OpenVMS or ancient GCC 2.x
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  build-aux/syntax-check.mk |   7 +-
>  src/hyperv/hyperv_driver.c|   2 +-
>  src/libvirt-domain.c  |   2 +-
>  src/nwfilter/nwfilter_ebiptables_driver.c |   2 +-
>  src/nwfilter/nwfilter_learnipaddr.c   |   2 +-
>  src/remote/remote_daemon_dispatch.c   |   2 +-
>  src/remote/remote_driver.c|   2 +-
>  src/util/Makefile.inc.am  |   1 +
>  src/util/virfile.c|   2 +-
>  src/util/virhostcpu.c |   2 +-
>  src/util/virintprops.h| 526 ++
>  src/util/virlog.c |   2 +-
>  src/util/virnetdevbridge.c|   2 +-
>  src/util/virpidfile.c |   2 +-
>  tests/virsystemdtest.c|   2 +-
>  tools/virsh-domain-monitor.c  |   2 +-
>  tools/virt-login-shell.c  |   7 +-
>  17 files changed, 548 insertions(+), 19 deletions(-)
>  create mode 100644 src/util/virintprops.h

From the intprops.h we use only these two macros, INT_MULTIPLY_OVERFLOW
and INT_BUFSIZE_BOUND.

The INT_MULTIPLY_OVERFLOW is used only for virDomainGetVcpuPinInfo and
virDomainGetVcpus APIs.  In both cases the multiplication result is used
to allocate some memory and stored as uint cpumap_len.

I guess that it should be safe to use g_uint_checked_mul() function.

For the INT_BUFSIZE_BOUND I was not able to find any replacement.
We can use the similar approach as you've did in virt-login-shell but
instead of magic number we can define a macro that would be large enough
to cover long long and unsigned long long variables.  Or we can use
dynamic allocation.

That way we wound not have to copy this code at all.

Pavel


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

Re: [libvirt] [PATCH 6/7] util: add virBufferTrimChars

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:40AM +0100, Ján Tomko wrote:
> A new helper for trimming combinations of specified characters from
> the tail of the buffer.
>
> Signed-off-by: Ján Tomko 
> ---
>
...

>  /**
>   * virBufferAddStr:
> diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
> index 38758a9125..183f78f279 100644
> --- a/src/util/virbuffer.h
> +++ b/src/util/virbuffer.h
> @@ -92,4 +92,5 @@ size_t virBufferGetIndent(const virBuffer *buf);
>  size_t virBufferGetEffectiveIndent(const virBuffer *buf);
>
>  void virBufferTrim(virBufferPtr buf, const char *trim, int len);
> +void virBufferTrimChars(virBufferPtr buf, const char *trim);
>  void virBufferAddStr(virBufferPtr buf, const char *str);
> diff --git a/tests/virbuftest.c b/tests/virbuftest.c
> index 1780b62bf4..7919075000 100644
> --- a/tests/virbuftest.c
> +++ b/tests/virbuftest.c
> @@ -12,6 +12,7 @@
>  struct testBufAddStrData {
>  const char *data;
>  const char *expect;
> +const char *arg;

Maybe *opaque? #naming,#bikeshedding...

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v4 2/7] virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback

[Marc Hartmayer]

On Fri, Dec 13, 2019 at 03:32 PM -0500, Cole Robinson  
wrote:
> On 12/12/19 8:46 AM, Marc Hartmayer wrote:
>> On Wed, Dec 11, 2019 at 08:11 PM -0500, Cole Robinson  
>> wrote:
>>> On 11/14/19 12:44 PM, Marc Hartmayer wrote:
 The commit 'close callback: move it to driver' (88f09b75eb99) moved
 the responsibility for the close callback to the driver. But if the
 driver doesn't support the connectRegisterCloseCallback API this
 function does nothing, even no unsupported error report. This behavior
 may lead to problems, for example memory leaks, as the caller cannot
 differentiate whether the close callback was 'really' registered or
 not.

>>>
>>>
>>> Full context:
>>> v1 subtrhead with jferlan and danpb:
>>> https://www.redhat.com/archives/libvir-list/2018-March/msg00906.html
>>> https://www.redhat.com/archives/libvir-list/2018-April/msg00116.html
>>>
>>> v2 subthread with john:
>>> https://www.redhat.com/archives/libvir-list/2018-April/msg02534.html
>>>
>>> My first thought is, why not just make this API start raising an error
>>> if it isn't supported?
>>>
>>> But you tried that here:
>>> https://www.redhat.com/archives/libvir-list/2018-March/msg00405.html
>> 
>> First, thanks for doing all the “history research”.
>> 
>>>
>>> I'm not really sure I buy the argument that we can't change the
>>> semantics of the API here. This is the only callback API that seems to
>>> not raise an explicit error. It's documented to raise an error. And
>>> there's possible memory leak due the ambiguity.
>> 
>> If we’re doing this so let’s fix the behavior of
>> 'virConnectUnregisterCloseCallback' as well.
>> 
>>>
>>> Yeah I see that virsh needs to be updated to match. In practice virsh
>>> shouldn't be a problem: this issue will only hit for local drivers, and
>>> virsh and client library will be updated together for that case.
>>>
>>> In theory if a python app is using this API and not expecting an
>>> exception, it could cause a regression. But it's also informing them
>>> that, hey, that connection callback you requested wasn't working in the
>>> first place. So it's arguably a correctness issue.
>>>
>>> So IMO we should be able to adjust this to return a proper error.
>>>
>>>
>>> BUT, if we stick with this direction, then we need to extend the doc
>>> text here to describe all of this:
>>>
>>> * Returns -1 if the driver can support close callback, but registering
>>> one failed. User must free opaque?
>>> * Returns 0 if the driver does not support close callback. We will free
>>> data for you
>>> * Returns 0 if the driver successfully registered a close callback. When
>>> that callback is triggered, opaque will be free'd
>>>
>>> But that sounds pretty nutty IMO :/
>> 
>> I know…
>
> I did a bit more digging. Even the virsh case isn't the biggest deal
> because CloseCallback failing is non-fatal. But like I mentioned before
> it shouldn't realistically be hit in practice because we can expect
> virsh and libvirt-client to be updated in lockstep.
>
> virt-manager, libguestfs, vdsm, kubevirt don't use this API
> nova does (nova/virt/libvirt/host.py) but it has code to catch the error
>
> So IMO this should be changed to have semantics like all the other event
> functions. Yes it's a semantic change, but I see it as explicitly
> erroring in a case that never actually worked. We've made changes like
> that many times, happens with XML validation semi regularly, and the
> UNDEFINE flag changes are other notable examples.
>
> danpb has your thinking changed on this? Previous discussion context is
> in this thread:
> https://www.redhat.com/archives/libvir-list/2018-April/msg00116.html
>
> Thanks,
> Cole
>

Polite ping.

-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


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

Re: [libvirt] [PATCH 5/7] virbuftest: use field names when initalizing test info

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:39AM +0100, Ján Tomko wrote:
> Allow adding new fields without changing all the macros.

It would be IMO worth mentioning the error your compiler gave you in the next
patch when you hadn't done this adjustment.

>
> Signed-off-by: Ján Tomko 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 4/7] virbuftest: declare testBufAddStrData earlier

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:38AM +0100, Ján Tomko wrote:
> Move the declaration to the beginning of the file for reuse.
>
> Signed-off-by: Ján Tomko 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 3/7] virbuftest: remove unnecessary labels

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:37AM +0100, Ján Tomko wrote:
> Remove the ret variables and labels from functions that no longer need
> them.
>
> Signed-off-by: Ján Tomko 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 2/7] virbuftest: use g_autofree

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:36AM +0100, Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 1/7] virbuftest: remove extra G_GNUC_UNUSED markers

[Erik Skultety]

On Tue, Jan 14, 2020 at 08:35:35AM +0100, Ján Tomko wrote:
> These functions do use the opaque argument.
>
> Signed-off-by: Ján Tomko 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH for 6.0.0] news: Document

[Andrea Bolognani]

On Tue, 2020-01-14 at 09:25 +0100, Michal Privoznik wrote:
> +  
> +
> +  qemu: Allow accessing NVMe disks directly
> +
> +
> +  Before this release there were two ways to configure a NVMe disk 
> for
> +  a domain.  The first was using disk/ with the 
> source/

Extra whitespace here.

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH for 6.0.0] news: Document

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 docs/news.xml | 16 
 1 file changed, 16 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index acb63709da..e9863dacf3 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -106,6 +106,22 @@
   hostname and the information stored in its TLS certificate.
 
   
+  
+
+  qemu: Allow accessing NVMe disks directly
+
+
+  Before this release there were two ways to configure a NVMe disk for
+  a domain.  The first was using disk/ with the source/
+  pointing to the /dev/nvme. The other was using PCI
+  assignment via hostdev/ element. Both have their
+  disadvantages: the former adds latency of file system and block
+  layers of the host kernel, the latter prohibits domain migration. In
+  this release the third way of configuring NVMe disk is added which
+  combines the advantages and drops disadvantages of the previous two
+  ways. It's accessible via disk type='nvme'/.
+
+  
 
 
   
-- 
2.24.1

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

Re: [libvirt] [PATCH] news: News for RDT-MBM feature

[Andrea Bolognani]

On Tue, 2020-01-14 at 13:48 +0800, Han Han wrote:
> +  
> +
> +  qemu: Support to report the stats of memory bandwidth usage
> +
> +
> +  Implement Intel RDT-MBM to libvirt. The stats can be got via
> +  virsh domstats --memory.
> +
> +  

Oops, sorry for missing this while updating the release notes, and
thanks for submitting the update yourself!

  Reviewed-by: Andrea Bolognani 

Tweaked it a bit and pushed.

-- 
Andrea Bolognani / Red Hat / Virtualization

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