Re: [PATCH v1 4/4] NEWS.rst: document the pSeries NVDIMM auto-alignment revival

[Daniel Henrique Barboza]

On 9/17/20 1:02 PM, Andrea Bolognani wrote:

On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:

+  * Re-introduce NVDIMM auto-alignment for pSeries Guests
+
+This feature was removed in libvirt v6.7.0 due to its shortcomings, namely
+the lack of consistency between domain XML and actual NVDIMM size the guest
+is using, but it came at a too high of a cost - we broke existing guests
+that were running in libvirt v.6.6.0 and now had to adjust the size by
+hand. The auto-alignment was re-introduced back, allowing existing guests
+to keep running as usual. But now, instead of auto-aligning in a 
transparent
+manner, it is also changing the domain XML. This brings a good balance
+between consistency of domain XML and guest information, and also relieves
+the user of knowing the alignment internals of pSeries NVDIMMs.


I feel like this is needlessly verbose. How about something like

   * Re-introduce NVDIMM auto-alignment for pSeries Guests

 The auto-alignment logic was removed in v6.7.0 in favor of
 requiring the size provided by the user to be already aligned;
 however, this had the unintended consequence of breaking some
 existing guests. v6.8.0 restores the previous behavior, and as an
 improvement upon it also reflects the auto-aligned value in the
 domain XML.

instead?



LGTM. I'll change it in v2.


Thanks,



DHB

Re: [PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes

[Daniel Henrique Barboza]

On 9/17/20 7:48 AM, Andrea Bolognani wrote:

On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:

In [1], changes were made to remove the existing auto-alignment
for pSeries NVDIMM devices. That design promotes strange situations
where the NVDIMM size reported in the domain XML is different
from what QEMU is actually using. We removed the auto-alignment
and relied on standard size validation.

However, this goes against Libvirt design philosophy of not
tampering with existing guest behavior, as pointed out by Daniel
in [2]. Since we can't know for sure whether there are guests that
are relying on the auto-alignment feature to work, the changes
made in [1] are a direct violation of this rule.

This patch reverts [1] entirely, re-enabling auto-alignment for
pSeries NVDIMM as it was before. Changes will be made to ease
the limitations of this design without hurting existing
guests.

This reverts the following commits:

- commit 0ee56369c8b4f2f898b6aa1ff1f51ab033be1c02,
"qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type".

- commit 07de813924caf37e535855541c0c1183d9d382e2,
"qemu_domain.c: do not auto-align ppc64 NVDIMMs"

- commit 0ccceaa57c50e5ee528f7073fa8723afd62b88b7,
"qemu_validate.c: add pSeries NVDIMM size alignment validation"

- commit 4fa2202d884414ad34d9952e72fb39b1d93c7e14,
"qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public"

- commit 2d93cbdea9d1b8dbf36bc0ffee6cb73d83d208c7,
"Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic""

[1] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html
[2] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html


Reverting multiple commits with a single commit is sort of unusual,
but in this case it seem to make sense and I don't really have a
problem with it, so unless someone shouts against this approach I
think we can merge it as-is.

A few tweaks to the commit message, though:

   * keep the order of commits consistent to the one they were merged
 in, which more specifically means putting 2d93cbdea9d1 at the
 very top of the list;

   * lose the commas after the various commit hashes - they break
 convenient double-click selection in most terminals;

   * indent the commit subject by two spaces for better readability.


Ok!



The commit is also missing your S-o-b, and as you know that's a
blocker for merging.


Ouch. reverting + squashing took its toll 



With the above addressed,

   Reviewed-by: Andrea Bolognani 

Re: [PATCH] libxl: use b_info->{acpi,acpi} when available

[Jim Fehlig]

On 9/16/20 3:09 AM, Michal Privoznik wrote:

On 9/10/20 6:18 AM, Marek Marczykowski-Górecki wrote:

b_info->u.hvm.{acpi,apic} are deprecated. But also, on recent libxl
version (4.14) the old one seems to be broken. While libxl part should
be fixed too, update the usage here and at some point drop support for
the old version.
b_info->acpi was added in Xen 4.8
b_info->apic was added in Xen 4.10
Xen 4.10 is the oldest version that still has security support (until
December 2020).

Signed-off-by: Marek Marczykowski-Górecki 
---
  src/libxl/libxl_conf.c  | 13 +
  tests/libxlxml2domconfigdata/basic-hvm.json |  4 ++--
  tests/libxlxml2domconfigdata/cpu-shares-hvm.json    |  4 ++--
  .../libxlxml2domconfigdata/fullvirt-acpi-slic.json  |  4 ++--
  .../fullvirt-cpuid-legacy-nest.json |  4 ++--
  tests/libxlxml2domconfigdata/fullvirt-cpuid.json    |  4 ++--
  .../max-eventchannels-hvm.json  |  4 ++--
  tests/libxlxml2domconfigdata/max-gntframes-hvm.json |  4 ++--
  tests/libxlxml2domconfigdata/moredevs-hvm.json  |  4 ++--
  .../libxlxml2domconfigdata/variable-clock-hvm.json  |  4 ++--
  .../vnuma-hvm-legacy-nest.json  |  4 ++--
  tests/libxlxml2domconfigdata/vnuma-hvm.json |  4 ++--
  12 files changed, 35 insertions(+), 22 deletions(-)


This looks good to me.

Reviewed-by: Michal Privoznik 

I'll wait a bit with pushing it though in case Jim wants to chime in.


It looks fine to me, so if you want it

Reviewed-by: Jim Fehlig 

On a slightly related note, it would be nice to bump the minimum supported 
LIBXL_API_VERSION in libvirt. Currently it is set to x040500. I'd like to bump 
it to 0x040800 (or perhaps higher). In fact, I have a downstream patch to do 
just that


https://build.opensuse.org/package/view_file/Virtualization/libvirt/suse-bump-xen-version.patch?expand=1

The problem is this API version was never advertised by libxl until Xen 4.13 
with commit c3999835df


http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c3999835df2d9917cf4b50be80be9a6358b1219d

We would need that commit backported to all downstream Xen packages that libvirt 
is expected to build against, which we've done at SUSE. But I can't expect that 
from all the other distros wired up to the CI. Suggestions welcome :-).


Regards,
Jim

[libvirt PATCH 2/2] news: dbus: use GLib implementation instead of libdbus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 NEWS.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index 2052fd700d..a2f7ecaf1d 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -28,6 +28,13 @@ v6.8.0 (unreleased)
 useful for containerised scenarios and can be used in both peer2peer and
 direct migrations.
 
+  * dbus: Use GLib implementation instead of libdbus
+
+Adopting GLib DBus implementation simplifies our code as libdbus provides
+low-level APIs where we had to have a lot of helper functions. With this
+change we also remove dependency on libdbus and possibly fix all the DBus
+related libvirtd crashes seen over the time.
+
 * **Bug fixes**
 
 * **Removed features**
-- 
2.26.2

[libvirt PATCH 0/2] Add news entries for HAL and GLib DBus

[Pavel Hrdina]

Pavel Hrdina (2):
  news: node_device: remove HAL node device backend
  news: dbus: use GLib implementation instead of libdbus

 NEWS.rst | 14 ++
 1 file changed, 14 insertions(+)

-- 
2.26.2

[libvirt PATCH 1/2] news: node_device: remove HAL node device backend

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 NEWS.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index 14f098b1f6..2052fd700d 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -30,6 +30,13 @@ v6.8.0 (unreleased)
 
 * **Bug fixes**
 
+* **Removed features**
+
+  * node_device: Remove HAL node device backend
+
+HAL is deprecated on all supported OS so there is no need to keep it
+in libvirt. udev backend is used on Linux OSes and devd can be eventually
+implemented as replacement for FreeBSD.
 
 v6.7.0 (2020-09-01)
 ===
-- 
2.26.2

Re: [libvirt PATCH 00/14] rewrite DBus to use GLib instead of libdbus

[Neal Gompa]

On Thu, Sep 17, 2020 at 4:31 AM Pavel Hrdina  wrote:
>
> Pavel Hrdina (14):
>   remove HAL node device driver
>   tests: mock libdbus in networkxml2firewalltest
>   util: introduce helpers for GLib DBus implementation
>   tests/virmock: extend number of arguments
>   tests: introduce virgdbusmock to mock GLib DBus functions
>   src/util/virpolkit: convert to use GLib DBus
>   src/util/virfirewalld: convert to use GLib DBus
>   src/util/virsystemd: convert to use GLib DBus
>   src/lxc/lxc_controller: convert to use GLib DBus
>   src/network/bridge_driver: convert to use GLib DBus
>   src/nwfilter/nwfilter_driver: convert to use GLib DBus
>   src/remote/remote_daemon: convert to use GLib DBus
>   src/rpc/virnetdaemon: convert to use GLib DBus
>   drop libdbus from libvirt
>
>  libvirt.spec.in  |3 -
>  meson.build  |   42 +-
>  meson_options.txt|2 -
>  po/POTFILES.in   |3 +-
>  src/libvirt_private.syms |   31 +-
>  src/libvirt_probes.d |6 -
>  src/lxc/lxc_controller.c |6 +-
>  src/lxc/meson.build  |1 -
>  src/meson.build  |1 -
>  src/network/bridge_driver.c  |   81 +-
>  src/network/meson.build  |2 -
>  src/node_device/meson.build  |5 -
>  src/node_device/node_device_driver.c |   10 +-
>  src/node_device/node_device_driver.h |5 -
>  src/node_device/node_device_hal.c|  843 
>  src/node_device/node_device_hal.h|   22 -
>  src/nwfilter/meson.build |1 -
>  src/nwfilter/nwfilter_driver.c   |  127 +-
>  src/remote/remote_daemon.c   |   73 +-
>  src/remote/remote_daemon_dispatch.c  |1 -
>  src/rpc/meson.build  |1 -
>  src/rpc/virnetdaemon.c   |  115 +-
>  src/util/meson.build |3 +-
>  src/util/virdbus.c   | 1871 --
>  src/util/virdbus.h   |   76 --
>  src/util/virdbuspriv.h   |   56 -
>  src/util/virfirewalld.c  |  197 ++-
>  src/util/virgdbus.c  |  425 ++
>  src/util/virgdbus.h  |   79 ++
>  src/util/virpolkit.c |  115 +-
>  src/util/virsystemd.c|  371 ++---
>  tests/meson.build|   29 +-
>  tests/networkxml2firewalltest.c  |   30 +-
>  tests/virdbusmock.c  |   62 -
>  tests/virdbustest.c  |  728 --
>  tests/virfirewalltest.c  |  154 +--
>  tests/virgdbusmock.c |   85 ++
>  tests/virmock.h  |   10 +-
>  tests/virpolkittest.c|  112 +-
>  tests/virsystemdtest.c   |  173 +--
>  40 files changed, 1330 insertions(+), 4627 deletions(-)
>  delete mode 100644 src/node_device/node_device_hal.c
>  delete mode 100644 src/node_device/node_device_hal.h
>  delete mode 100644 src/util/virdbus.c
>  delete mode 100644 src/util/virdbus.h
>  delete mode 100644 src/util/virdbuspriv.h
>  create mode 100644 src/util/virgdbus.c
>  create mode 100644 src/util/virgdbus.h
>  delete mode 100644 tests/virdbusmock.c
>  delete mode 100644 tests/virdbustest.c
>  create mode 100644 tests/virgdbusmock.c
>
> --
> 2.26.2
>

Reviewed-by: Neal Gompa 

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

Re: [PATCH v1 4/4] NEWS.rst: document the pSeries NVDIMM auto-alignment revival

[Andrea Bolognani]

On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:
> +  * Re-introduce NVDIMM auto-alignment for pSeries Guests
> +
> +This feature was removed in libvirt v6.7.0 due to its shortcomings, 
> namely
> +the lack of consistency between domain XML and actual NVDIMM size the 
> guest
> +is using, but it came at a too high of a cost - we broke existing guests
> +that were running in libvirt v.6.6.0 and now had to adjust the size by
> +hand. The auto-alignment was re-introduced back, allowing existing guests
> +to keep running as usual. But now, instead of auto-aligning in a 
> transparent
> +manner, it is also changing the domain XML. This brings a good balance
> +between consistency of domain XML and guest information, and also 
> relieves
> +the user of knowing the alignment internals of pSeries NVDIMMs.

I feel like this is needlessly verbose. How about something like

  * Re-introduce NVDIMM auto-alignment for pSeries Guests

The auto-alignment logic was removed in v6.7.0 in favor of
requiring the size provided by the user to be already aligned;
however, this had the unintended consequence of breaking some
existing guests. v6.8.0 restores the previous behavior, and as an
improvement upon it also reflects the auto-aligned value in the
domain XML.

instead?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v1 3/4] domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefParseXML()

[Andrea Bolognani]

On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:
> @@ -16981,23 +17000,11 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
> +if (virDomainNVDimmAlignSizePseries(def) < 0)
> +goto error;

The outcome of this change is good, and overall it doesn't negatively
impact existing guest; however, the ParseXML() function is not the
correct place to perform it, and it should happen in the PostParse()
callback instead.

If the restrictions on alignment are, as you claim in the commit
message for the patch before this one, not QEMU-specific, then a
reasonable home for the rounding logic could be
virDomainDefPostParseMemory(), but you could also leave it in the
QEMU driver by implementing a qemuDomainMemoryDefPostParse() function
that's called by the qemuDomainDeviceDefPostParse() dispatcher.

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH 6/9] Jailhouse driver: Implementation of DomainLookup* callbacks

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 src/jailhouse/jailhouse_driver.c | 70 
 1 file changed, 61 insertions(+), 9 deletions(-)

diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
index 2bb249f996..8c84a23388 100644
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -292,25 +292,77 @@ jailhouseConnectListAllDomains(virConnectPtr conn,
 static virDomainPtr
 jailhouseDomainLookupByID(virConnectPtr conn, int id)
 {
-UNUSED(conn);
-UNUSED(id);
-return NULL;
+virJailhouseDriverPtr driver = conn->privateData;
+virDomainObjPtr cell;
+virDomainPtr dom = NULL;
+
+cell = virDomainObjListFindByID(driver->domains, id);
+
+if (!cell) {
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("No domain with matching ID '%d'"), id);
+goto cleanup;
+}
+
+if (virDomainLookupByIDEnsureACL(conn, cell->def) < 0)
+goto cleanup;
+
+dom = virGetDomain(conn, cell->def->name, cell->def->uuid, cell->def->id);
+ cleanup:
+virDomainObjEndAPI();
+return dom;
 }
 
 static virDomainPtr
 jailhouseDomainLookupByName(virConnectPtr conn, const char *name)
 {
-UNUSED(conn);
-UNUSED(name);
-return NULL;
+virJailhouseDriverPtr driver = conn->privateData;
+virDomainObjPtr cell;
+virDomainPtr dom = NULL;
+
+cell = virDomainObjListFindByName(driver->domains, name);
+
+if (!cell) {
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("No domain with matching name '%s'"), name);
+goto cleanup;
+}
+
+if (virDomainLookupByNameEnsureACL(conn, cell->def) < 0)
+goto cleanup;
+
+dom = virGetDomain(conn, cell->def->name, cell->def->uuid, cell->def->id);
+
+ cleanup:
+virDomainObjEndAPI();
+return dom;
 }
 
 static virDomainPtr
 jailhouseDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
 {
-UNUSED(conn);
-UNUSED(uuid);
-return NULL;
+virJailhouseDriverPtr driver = conn->privateData;
+virDomainObjPtr cell;
+virDomainPtr dom = NULL;
+
+cell = virDomainObjListFindByUUID(driver->domains, uuid);
+
+if (!cell) {
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(uuid, uuidstr);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("No domain with matching UUID '%s'"), uuidstr);
+goto cleanup;
+}
+
+if (virDomainLookupByUUIDEnsureACL(conn, cell->def) < 0)
+goto cleanup;
+
+dom = virGetDomain(conn, cell->def->name, cell->def->uuid, cell->def->id);
+
+ cleanup:
+virDomainObjEndAPI();
+return dom;
 }
 
 static virDomainObjPtr
-- 
2.26.2

[libvirt PATCH 7/9] Jailhouse driver: Implementation of DomainShutdown/Destroy callbacks

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 src/jailhouse/jailhouse_api.c|   8 +++
 src/jailhouse/jailhouse_driver.c | 101 +--
 2 files changed, 104 insertions(+), 5 deletions(-)

diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
index 783903e939..510e2f5f66 100644
--- a/src/jailhouse/jailhouse_api.c
+++ b/src/jailhouse/jailhouse_api.c
@@ -71,6 +71,14 @@ int cell_match(const struct dirent *dirent);
 
 int createCell(const char *conf_file);
 
+int loadImagesInCell(virJailhouseCellId cell_id, char *images, int num_images);
+
+int shutdownCell(virJailhouseCellId cell_id);
+
+int startCell(virJailhouseCellId cell_id);
+
+int destroyCell(virJailhouseCellId cell_id);
+
 int getCellInfo(const unsigned int id,
 virJailhouseCellInfoPtr * cell_info);
 
diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
index 8c84a23388..46c7759cb8 100644
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -406,8 +406,8 @@ jailhouseDomainCreateWithFlags(virDomainPtr domain,
unsigned int flags)
 {
 virJailhouseDriverPtr driver = domain->conn->privateData;
-virDomainObjPtr cell;
 virJailhouseCellInfoPtr cell_info;
+virDomainObjPtr cell;
 int ret = -1;
 
 virCheckFlags(VIR_DOMAIN_NONE, -1);
@@ -566,18 +566,107 @@ jailhouseDomainCreateXML(virConnectPtr conn,
 return dom;
 }
 
+static int
+jailhouseDomainShutdownFlags(virDomainPtr domain, unsigned int flags)
+{
+virJailhouseDriverPtr driver = domain->conn->privateData;
+virJailhouseCellInfoPtr cell_info;
+virDomainObjPtr cell;
+virJailhouseCellId cell_id;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(cell = virJailhouseDomObjFromDomain(domain)))
+goto cleanup;
+
+if (virDomainShutdownFlagsEnsureACL(domain->conn, cell->def, flags) < 0)
+goto cleanup;
+
+if (virDomainObjGetState(cell, NULL) != VIR_DOMAIN_RUNNING)
+goto cleanup;
+
+if (!(cell_info = virJailhouseFindCellByName(driver, cell->def->name))) {
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching name %s and ID %d)"),
+   cell->def->name, cell->def->id);
+virDomainObjListRemove(driver->domains, cell);
+goto cleanup;
+}
+
+// Initialize the cell_id.
+cell_id.id = cell->def->id;
+cell_id.padding = 0;
+if (virStrcpy(cell_id.name, cell->def->name, JAILHOUSE_CELL_ID_NAMELEN) < 
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Cell name %s length exceeded the limit"),
+   cell->def->name);
+goto cleanup;
+}
+
+if (shutdownCell(cell_id) < 0)
+goto cleanup;
+
+virDomainObjSetState(cell, VIR_DOMAIN_SHUTOFF, 
VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
 static int
 jailhouseDomainShutdown(virDomainPtr domain)
 {
-UNUSED(domain);
-return -1;
+return jailhouseDomainShutdownFlags(domain, 0);
+}
+
+static int
+jailhouseDomainDestroyFlags(virDomainPtr domain, unsigned int flags)
+{
+virJailhouseDriverPtr driver = domain->conn->privateData;
+virJailhouseCellInfoPtr cell_info;
+virDomainObjPtr cell;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(cell = virJailhouseDomObjFromDomain(domain)))
+goto cleanup;
+
+if (virDomainDestroyFlagsEnsureACL(domain->conn, cell->def) < 0)
+goto cleanup;
+
+if (virDomainObjGetState(cell, NULL) != VIR_DOMAIN_SHUTOFF) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("Domain %s is still running."),
+   cell->def->name);
+goto cleanup;
+}
+
+if (!(cell_info = virJailhouseFindCellByName(driver, cell->def->name))) {
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching name %s and ID %d)"),
+   cell->def->name, cell->def->id);
+virDomainObjListRemove(driver->domains, cell);
+goto cleanup;
+}
+
+// Remove the cell from the domain list.
+virDomainObjListRemove(driver->domains, cell);
+
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
 }
 
 static int
 jailhouseDomainDestroy(virDomainPtr domain)
 {
-UNUSED(domain);
-return -1;
+return jailhouseDomainDestroyFlags(domain, 0);
 }
 
 static int
@@ -675,7 +764,9 @@ static virHypervisorDriver jailhouseHypervisorDriver = {
 .domainCreateWithFlags = jailhouseDomainCreateWithFlags,/* 6.3.0 */
 .domainCreateXML = jailhouseDomainCreateXML, /* 6.3.0 */
 .domainShutdown = jailhouseDomainShutdown,  /* 6.3.0 */
+.domainShutdownFlags = jailhouseDomainShutdownFlags,  /* 6.3.0 */
 .domainDestroy = jailhouseDomainDestroy,/* 6.3.0 */
+.domainDestroyFlags = jailhouseDomainDestroyFlags,/* 6.3.0 */
 .domainGetInfo = 

[libvirt PATCH 9/9] Jailhouse driver: Add events to events queue in DomainCreate/Shutdown/Destroy

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 src/jailhouse/jailhouse_driver.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
index 45b1f35896..70f0f365cb 100644
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -122,6 +122,7 @@ jailhouseCreateAndLoadCells(virJailhouseDriverPtr driver)
 // Create all cells in the hypervisor.
 if (createJailhouseCells(driver->config->cell_config_dir) < 0)
 return -1;
+
 // Get all cells created above.
 driver->cell_info_list = getJailhouseCellsInfo();
 
@@ -137,6 +138,7 @@ jailhouseFreeDriver(virJailhouseDriverPtr driver)
 virMutexDestroy(>lock);
 virObjectUnref(driver->xmlopt);
 virObjectUnref(driver->domains);
+virObjectUnref(driver->domainEventState);
 virObjectUnref(driver->config);
 VIR_FREE(driver);
 }
@@ -222,6 +224,9 @@ jailhouseStateInitialize(bool privileged G_GNUC_UNUSED,
 if (!(jailhouse_driver->domains = virDomainObjListNew()))
 goto error;
 
+if (!(jailhouse_driver->domainEventState = virObjectEventStateNew()))
+goto error;
+
 if (!(cfg = virJailhouseDriverConfigNew()))
 goto error;
 
@@ -476,6 +481,7 @@ jailhouseDomainCreateXML(virConnectPtr conn,
 virDomainDefPtr def = NULL;
 virDomainObjPtr cell = NULL;
 virJailhouseCellId cell_id;
+virObjectEventPtr event = NULL;
 char **images = NULL;
 int num_images = 0, i = 0;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
@@ -573,12 +579,16 @@ jailhouseDomainCreateXML(virConnectPtr conn,
 
 dom = virGetDomain(conn, cell->def->name, cell->def->uuid, cell->def->id);
 
+event = virDomainEventLifecycleNewFromObj(cell,
+  VIR_DOMAIN_EVENT_STARTED,
+  VIR_DOMAIN_EVENT_STARTED_BOOTED);
  cleanup:
 if (!dom && removeInactive && !cell->persistent)
 virDomainObjListRemove(driver->domains, cell);
 
 virDomainDefFree(def);
 virDomainObjEndAPI();
+virObjectEventStateQueue(driver->domainEventState, event);
 return dom;
 }
 
@@ -589,6 +599,7 @@ jailhouseDomainShutdownFlags(virDomainPtr domain, unsigned 
int flags)
 virJailhouseCellInfoPtr cell_info;
 virDomainObjPtr cell;
 virJailhouseCellId cell_id;
+virObjectEventPtr event = NULL;
 int ret = -1;
 
 virCheckFlags(0, -1);
@@ -627,8 +638,12 @@ jailhouseDomainShutdownFlags(virDomainPtr domain, unsigned 
int flags)
 
 ret = 0;
 
+event = virDomainEventLifecycleNewFromObj(cell,
+  VIR_DOMAIN_EVENT_STOPPED,
+  
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
  cleanup:
 virDomainObjEndAPI();
+virObjectEventStateQueue(driver->domainEventState, event);
 return ret;
 }
 
@@ -644,6 +659,7 @@ jailhouseDomainDestroyFlags(virDomainPtr domain, unsigned 
int flags)
 virJailhouseDriverPtr driver = domain->conn->privateData;
 virJailhouseCellInfoPtr cell_info;
 virDomainObjPtr cell;
+virObjectEventPtr event = NULL;
 int ret = -1;
 
 virCheckFlags(0, -1);
@@ -674,8 +690,12 @@ jailhouseDomainDestroyFlags(virDomainPtr domain, unsigned 
int flags)
 
 ret = 0;
 
+event = virDomainEventLifecycleNewFromObj(cell,
+  VIR_DOMAIN_EVENT_STOPPED,
+  
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
  cleanup:
 virDomainObjEndAPI();
+virObjectEventStateQueue(driver->domainEventState, event);
 return ret;
 }
 
-- 
2.26.2

[libvirt PATCH 4/9] Jailhouse driver: Implementation of DomainCreate* callbacks

[Daniel P . Berrangé]

From: Prakhar Bansal 

Implemented Jailhouse hypervisor APIs for cell
load/start/shutdown/destroy.
---
 src/jailhouse/jailhouse_api.c| 100 --
 src/jailhouse/jailhouse_api.h|  29 +
 src/jailhouse/jailhouse_driver.c | 217 +--
 src/jailhouse/jailhouse_driver.h |   8 ++
 4 files changed, 335 insertions(+), 19 deletions(-)

diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
index cda00b50e7..783903e939 100644
--- a/src/jailhouse/jailhouse_api.c
+++ b/src/jailhouse/jailhouse_api.c
@@ -43,11 +43,14 @@
 #define JAILHOUSE_CELLS "/sys/devices/jailhouse/cells"
 #define MAX_JAILHOUSE_SYS_CONFIG_FILE_SIZE  1024*1024
 #define MAX_JAILHOUSE_CELL_CONFIG_FILE_SIZE 1024
+#define MAX_JAILHOUSE_CELL_IMAGE_FILE_SIZE  64*1024*1024
 
 
 #define JAILHOUSE_ENABLE   _IOW(0, 0, void *)
 #define JAILHOUSE_DISABLE  _IO(0, 1)
 #define JAILHOUSE_CELL_CREATE  _IOW(0, 2, virJailhouseCellCreate)
+#define JAILHOUSE_CELL_LOAD_IOW(0, 3, virJailhouseCellLoad)
+#define JAILHOUSE_CELL_START   _IOW(0, 4, virJailhouseCellId)
 #define JAILHOUSE_CELL_DESTROY _IOW(0, 5, virJailhouseCellId)
 
 #define VIR_FROM_THIS VIR_FROM_JAILHOUSE
@@ -68,8 +71,6 @@ int cell_match(const struct dirent *dirent);
 
 int createCell(const char *conf_file);
 
-int destroyCell(virJailhouseCellId cell_id);
-
 int getCellInfo(const unsigned int id,
 virJailhouseCellInfoPtr * cell_info);
 
@@ -254,7 +255,7 @@ readSysfsCellString(const unsigned int id, const char 
*entry)
 }
 
 int
-getCellInfo(const unsigned int id, virJailhouseCellInfoPtr * cell_info_ptr)
+getCellInfo(const unsigned int id, virJailhouseCellInfoPtr *cell_info_ptr)
 {
 char *tmp;
 
@@ -345,28 +346,105 @@ getJailhouseCellsInfo(void)
 }
 
 int
-destroyCell(virJailhouseCellId cell_id)
+loadImagesInCell(virJailhouseCellId cell_id, char **images, int num_images)
+{
+   virJailhousePreloadImagePtr image;
+   virJailhouseCellLoadPtr cell_load;
+   g_autofree char *buffer = NULL;
+   unsigned int n;
+   int len = -1, err = -1;
+   VIR_AUTOCLOSE fd = -1;
+
+
+   if (VIR_ALLOC(cell_load) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s",
+   _("Insufficient memory for cell load"));
+return -1;
+   }
+
+
+   if (VIR_ALLOC_N(cell_load->image, num_images) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s",
+   _("Insufficient memory for cell load images"));
+return -1;
+   }
+
+   cell_load->id = cell_id;
+   cell_load->num_preload_images = num_images;
+
+   for (n = 0, image = cell_load->image; n < num_images; n++, image++) {
+len = virFileReadAll(images[n], MAX_JAILHOUSE_CELL_IMAGE_FILE_SIZE, 
);
+if (len < 0 || !buffer) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("Failed to read the image file %s"),
+images[n]);
+ return -1;
+}
+
+image->source_address = (unsigned long)buffer;
+image->size = len;
+
+// TODO(Prakhar): Add support for target address.
+image->target_address = 0;
+   }
+
+   fd = openDev();
+
+   err = ioctl(fd, JAILHOUSE_CELL_LOAD, cell_load);
+   if (err) {
+   virReportSystemError(errno,
+_("Loading cell images for %d failed"),
+cell_id.id);
+   return -1;
+   }
+
+   return 0;
+}
+
+int
+shutdownCell(virJailhouseCellId cell_id)
+{
+// Loading 0 images in the cell causes cell to shutdown.
+return loadImagesInCell(cell_id, NULL, 0);
+}
+
+int
+startCell(virJailhouseCellId cell_id)
 {
 int err = -1;
 VIR_AUTOCLOSE fd = -1;
 
 fd = openDev();
 
-err = ioctl(fd, JAILHOUSE_CELL_DESTROY, _id);
-if (err)
+err = ioctl(fd, JAILHOUSE_CELL_START, _id);
+if (err) {
 virReportSystemError(errno,
- _("Destroying cell %d failed"),
+ _("Start cell %d failed"),
  cell_id.id);
+return -1;
+}
 
-return err;
+return 0;
 }
 
 int
-destroyJailhouseCells(virJailhouseCellInfoPtr *cell_info_list G_GNUC_UNUSED)
+destroyCell(virJailhouseCellId cell_id)
 {
+int err = -1;
+VIR_AUTOCLOSE fd = -1;
+
+fd = openDev();
 
-/* Iterate over all cells in cell_info_list and destroy each cell */
-// TODO: Not implemented yet.
+err = ioctl(fd, JAILHOUSE_CELL_DESTROY, _id);
+if (err) {
+virReportSystemError(errno,
+ _("Destroying cell %d failed"),
+ cell_id.id);
+
+return -1;
+}
 
 return 0;
 }
diff --git a/src/jailhouse/jailhouse_api.h b/src/jailhouse/jailhouse_api.h
index 8362cb3d0f..ba39a4c8b7 100644
--- a/src/jailhouse/jailhouse_api.h
+++ b/src/jailhouse/jailhouse_api.h
@@ -48,6 

[libvirt PATCH 8/9] Jailhouse driver: Fixes for creation of cells, fetching cell info, disabling jailhouse hypervisor

[Daniel P . Berrangé]

From: Prakhar Bansal 

- Added xmlopt to the Jailhouse driver
- Added ACL check in ConnectOpen
---
 src/jailhouse/jailhouse_api.c| 48 +-
 src/jailhouse/jailhouse_driver.c | 58 
 2 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
index 510e2f5f66..bb82b5a31e 100644
--- a/src/jailhouse/jailhouse_api.c
+++ b/src/jailhouse/jailhouse_api.c
@@ -69,15 +69,9 @@ char *readSysfsCellString(const unsigned int id, const char 
*entry);
 
 int cell_match(const struct dirent *dirent);
 
-int createCell(const char *conf_file);
-
-int loadImagesInCell(virJailhouseCellId cell_id, char *images, int num_images);
-
-int shutdownCell(virJailhouseCellId cell_id);
+int cell_match_info(const struct dirent *dirent);
 
-int startCell(virJailhouseCellId cell_id);
-
-int destroyCell(virJailhouseCellId cell_id);
+int createCell(const char *conf_file);
 
 int getCellInfo(const unsigned int id,
 virJailhouseCellInfoPtr * cell_info);
@@ -121,25 +115,31 @@ jailhouseDisable(void)
 fd = openDev();
 
 err = ioctl(fd, JAILHOUSE_DISABLE);
-if (err)
+if (err) {
 virReportSystemError(errno,
  "%s",
  _("Failed to disable jailhouse: %s"));
+ return -1;
+}
 
 VIR_DEBUG("Jailhouse hypervisor is disabled");
 
-return err;
+return 0;
 }
 
 int
 cell_match(const struct dirent *dirent)
 {
 char *ext = strrchr(dirent->d_name, '.');
-
 return dirent->d_name[0] != '.'
-&& (STREQ(ext, JAILHOUSE_CELL_FILE_EXTENSION) == 0);
+&& STREQ(ext, JAILHOUSE_CELL_FILE_EXTENSION);
 }
 
+int
+cell_match_info(const struct dirent *dirent)
+{
+return dirent->d_name[0] != '.';
+}
 int
 createJailhouseCells(const char *dir_path)
 {
@@ -150,7 +150,6 @@ createJailhouseCells(const char *dir_path)
 
 if (strlen(dir_path) == 0)
 return ret;
-
 num_entries = scandir(dir_path, , cell_match, alphasort);
 if (num_entries == -1) {
 if (errno == ENOENT) {
@@ -170,7 +169,8 @@ createJailhouseCells(const char *dir_path)
 for (i = 0; i < num_entries; i++) {
 g_autofree char *file_path = g_strdup_printf("%s/%s", dir_path, 
namelist[i]->d_name);
 
-if (createCell(file_path) != 0) {
+
+if (createCell(file_path) < 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Cell creation failed with conf found in  %s."),
namelist[i]->d_name);
@@ -208,13 +208,13 @@ createCell(const char *conf_file)
 VIR_AUTOCLOSE fd = -1;
 
 if (strlen(conf_file) == 0)
-return err;
+return -1;
 
 len = virFileReadAll(conf_file, MAX_JAILHOUSE_CELL_CONFIG_FILE_SIZE, 
);
 if (len < 0 || !buffer) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   "%s", _("Failed to read the system configuration file"));
-return err;
+return -1;
 }
 
 cell_create.config_address = (unsigned long) buffer;
@@ -223,12 +223,14 @@ createCell(const char *conf_file)
 fd = openDev();
 
 err = ioctl(fd, JAILHOUSE_CELL_CREATE, _create);
-if (err)
+if (err) {
 virReportSystemError(errno,
  "%s",
  _("Cell creation failed: %s"));
+return -1;
+}
 
-return err;
+return 0;
 }
 
 void
@@ -243,11 +245,11 @@ cellInfoFree(virJailhouseCellInfoPtr cell_info)
 char *
 readSysfsCellString(const unsigned int id, const char *entry)
 {
-g_autofree char *buffer = NULL;
+char *buffer = NULL;
 g_autofree char *file_path = NULL;
 int len = -1;
 
-file_path = g_strdup_printf(JAILHOUSE_CELLS "%u/%s", id, entry);
+file_path = g_strdup_printf(JAILHOUSE_CELLS "/%u/%s", id, entry);
 
 len = virFileReadAll(file_path, 1024, );
 if (len < 0 || !buffer) {
@@ -277,13 +279,12 @@ getCellInfo(const unsigned int id, 
virJailhouseCellInfoPtr *cell_info_ptr)
 
 /* get cell name */
 tmp = readSysfsCellString(id, "name");
-if (virStrncpy(cell_info->id.name, tmp, JAILHOUSE_CELL_ID_NAMELEN, 
JAILHOUSE_CELL_ID_NAMELEN) < 0) {
+if (virStrcpy(cell_info->id.name, tmp, JAILHOUSE_CELL_ID_NAMELEN) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cell ID %s too long to be copied to the cell info"),
tmp);
 return -1;
 }
-
 cell_info->id.name[JAILHOUSE_CELL_ID_NAMELEN] = 0;
 VIR_FREE(tmp);
 
@@ -310,8 +311,7 @@ getJailhouseCellsInfo(void)
 int num_entries;
 size_t i;
 
-num_entries =
-scandir(JAILHOUSE_CELLS, , cell_match, alphasort);
+num_entries = scandir(JAILHOUSE_CELLS, , cell_match_info, 
alphasort);
 if (num_entries == -1) {
 if (errno == ENOENT) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/jailhouse/jailhouse_driver.c 

[libvirt PATCH 1/9] Jailhouse driver: first commit with skeleton code

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 include/libvirt/virterror.h  |   2 +-
 libvirt.spec.in  |   7 +
 m4/virt-driver-jailhouse.m4  |  42 +
 meson.build  |   4 +
 meson_options.txt|   1 +
 src/conf/domain_conf.c   |   1 +
 src/conf/domain_conf.h   |   1 +
 src/jailhouse/Makefile.inc.am|  21 +++
 src/jailhouse/jailhouse_driver.c | 219 +++
 src/jailhouse/jailhouse_driver.h |  23 +++
 src/jailhouse/libvirtd_jailhouse.aug |  43 ++
 src/jailhouse/meson.build|  48 ++
 src/libvirt.c|  10 ++
 src/meson.build  |   1 +
 src/qemu/qemu_command.c  |   1 +
 src/util/virerror.c  |   1 +
 16 files changed, 424 insertions(+), 1 deletion(-)
 create mode 100644 m4/virt-driver-jailhouse.m4
 create mode 100644 src/jailhouse/Makefile.inc.am
 create mode 100644 src/jailhouse/jailhouse_driver.c
 create mode 100644 src/jailhouse/jailhouse_driver.h
 create mode 100644 src/jailhouse/libvirtd_jailhouse.aug
 create mode 100644 src/jailhouse/meson.build

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 0f1c32283d..97f2ac16d8 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -136,7 +136,7 @@ typedef enum {
 
 VIR_FROM_TPM = 70,  /* Error from TPM */
 VIR_FROM_BPF = 71,  /* Error from BPF code */
-
+VIR_FROM_JAILHOUSE = 72,/* Error from Jailhouse driver */
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
 # endif
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 62b401bd08..ca65063e79 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -17,6 +17,7 @@
 %define with_lxc   0%{!?_without_lxc:1}
 %define with_libxl 0%{!?_without_libxl:1}
 %define with_vbox  0%{!?_without_vbox:1}
+%define with_jailhouse 0%{!?_without_jailhouse:1}
 
 %define with_qemu_tcg  %{with_qemu}
 
@@ -1043,6 +1044,12 @@ exit 1
 %define arg_vmware -Ddriver_vmware=disabled
 %endif
 
+%if %{with_jailhouse}
+%define arg_jailhouse --with-jailhouse
+%else
+%define arg_jailhouse --without-jailhouse
+%endif
+
 %if %{with_storage_rbd}
 %define arg_storage_rbd -Dstorage_rbd=enabled
 %else
diff --git a/m4/virt-driver-jailhouse.m4 b/m4/virt-driver-jailhouse.m4
new file mode 100644
index 00..9008c6ce30
--- /dev/null
+++ b/m4/virt-driver-jailhouse.m4
@@ -0,0 +1,42 @@
+dnl The Jailhouse driver
+dnl
+dnl Copyright (C) 2016 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+dnl
+
+AC_DEFUN([LIBVIRT_DRIVER_ARG_JAILHOUSE], [
+  LIBVIRT_ARG_WITH_FEATURE([JAILHOUSE], [Jailhouse], [check])
+])
+
+AC_DEFUN([LIBVIRT_DRIVER_CHECK_JAILHOUSE], [
+  if test "$with_jailhouse" = "check"; then
+with_jailhouse=$with_linux
+  fi
+
+  if test "$with_jailhouse" = "yes" && test "$with_linux" = "no"; then
+AC_MSG_ERROR([The Jailhouse driver can be enabled on Linux only.])
+  fi
+
+  if test "$with_jailhouse" = "yes"; then
+AC_DEFINE_UNQUOTED([WITH_JAILHOUSE], 1, [whether Jailhouse driver is 
enabled])
+  fi
+
+  AM_CONDITIONAL([WITH_JAILHOUSE], [test "$with_jailhouse" = "yes"])
+])
+
+AC_DEFUN([LIBVIRT_DRIVER_RESULT_JAILHOUSE], [
+  LIBVIRT_RESULT([Jailhouse], [$with_jailhouse])
+])
diff --git a/meson.build b/meson.build
index 195d7cd784..f4f9ca4119 100644
--- a/meson.build
+++ b/meson.build
@@ -1895,6 +1895,10 @@ elif get_option('secdriver_selinux').enabled()
   error('You must install the libselinux development package in order to 
compile libvirt.')
 endif
 
+if get_option('driver_jailhouse').enabled()
+  conf.set('WITH_JAILHOUSE', 1)
+endif
+
 if conf.has('WITH_QEMU') or conf.has('WITH_LXC') or conf.has('WITH_NETWORK')
   conf.set('WITH_BRIDGE', 1)
 endif
diff --git a/meson_options.txt b/meson_options.txt
index 7838630c1e..4f237be5dc 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -69,6 +69,7 @@ option('driver_vbox', type: 'feature', value: 'enabled', 
description: 'VirtualBo
 option('vbox_xpcomc_dir', type: 'string', value: '', description: 'Location of 
directory containing VirtualBox XPCOMC library')
 option('driver_vmware', type: 'feature', value: 'enabled', description: 
'VMware driver')
 option('driver_vz', type: 'feature', value: 'auto', 

[libvirt PATCH 2/9] Jailhouse driver: Implementation of ConnectOpen

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 include/libvirt/virterror.h |   1 +
 po/POTFILES.in  |   2 +
 src/jailhouse/Makefile.inc.am   |  34 ++-
 src/jailhouse/jailhouse.conf|  10 +
 src/jailhouse/jailhouse_api.c   | 372 
 src/jailhouse/jailhouse_api.h   |  74 ++
 src/jailhouse/jailhouse_driver.c| 302 +-
 src/jailhouse/jailhouse_driver.h|  51 
 src/jailhouse/meson.build   |   1 +
 src/libvirt.c   |  10 -
 src/remote/remote_daemon.c  |   4 +
 src/remote/remote_daemon_dispatch.c |   3 +-
 12 files changed, 783 insertions(+), 81 deletions(-)
 create mode 100644 src/jailhouse/jailhouse.conf
 create mode 100644 src/jailhouse/jailhouse_api.c
 create mode 100644 src/jailhouse/jailhouse_api.h

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 97f2ac16d8..9f1bca2684 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -137,6 +137,7 @@ typedef enum {
 VIR_FROM_TPM = 70,  /* Error from TPM */
 VIR_FROM_BPF = 71,  /* Error from BPF code */
 VIR_FROM_JAILHOUSE = 72,/* Error from Jailhouse driver */
+
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
 # endif
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 471af30b89..317db58a21 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -85,6 +85,8 @@
 @SRCDIR@src/interface/interface_backend_netcf.c
 @SRCDIR@src/interface/interface_backend_udev.c
 @SRCDIR@src/internal.h
+@SRCDIR@src/jailhouse/jailhouse_api.c
+@SRCDIR@src/jailhouse/jailhouse_driver.c
 @SRCDIR@src/libvirt-domain-checkpoint.c
 @SRCDIR@src/libvirt-domain-snapshot.c
 @SRCDIR@src/libvirt-domain.c
diff --git a/src/jailhouse/Makefile.inc.am b/src/jailhouse/Makefile.inc.am
index 02822b2ea1..324c3b1b16 100644
--- a/src/jailhouse/Makefile.inc.am
+++ b/src/jailhouse/Makefile.inc.am
@@ -1,8 +1,11 @@
 # vim: filetype=automake
 
+
 JAILHOUSE_DRIVER_SOURCES = \
jailhouse/jailhouse_driver.c \
jailhouse/jailhouse_driver.h \
+   jailhouse/jailhouse_api.c \
+   jailhouse/jailhouse_api.h \
$(NULL)
 
 
@@ -11,11 +14,34 @@ DRIVER_SOURCE_FILES += $(addprefix 
$(srcdir)/,$(JAILHOUSE_DRIVER_SOURCES))
 EXTRA_DIST += $(JAILHOUSE_DRIVER_SOURCES)
 
 if WITH_JAILHOUSE
-noinst_LTLIBRARIES += libvirt_driver_jailhouse.la
-libvirt_la_BUILT_LIBADD += libvirt_driver_jailhouse.la
-libvirt_driver_jailhouse_la_CFLAGS = \
+
+noinst_LTLIBRARIES += libvirt_driver_jailhouse_impl.la
+libvirt_driver_jailhouse_la_SOURCES =
+libvirt_driver_jailhouse_la_LIBADD = \
+   libvirt_driver_jailhouse_impl.la \
+   libvirt.la \
+   $(GLIB_LIBS) \
+   $(NULL)
+mod_LTLIBRARIES += libvirt_driver_jailhouse.la
+libvirt_driver_jailhouse_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF)
+
+libvirt_driver_jailhouse_impl_la_CFLAGS = \
-I$(srcdir)/conf \
$(AM_CFLAGS) \
$(NULL)
-libvirt_driver_jailhouse_la_SOURCES = $(JAILHOUSE_DRIVER_SOURCES)
+libvirt_driver_jailhouse_impl_la_SOURCES = \
+   $(JAILHOUSE_DRIVER_SOURCES)
+
+sbin_PROGRAMS += virtjailhoused
+
+virtjailhoused_SOURCES = $(REMOTE_DAEMON_SOURCES)
+nodist_virtjailhoused_SOURCES = $(REMOTE_DAEMON_GENERATED)
+virtjailhoused_CFLAGS = \
+   $(REMOTE_DAEMON_CFLAGS) \
+   -DDAEMON_NAME="\"virtjailhoused\"" \
+   -DMODULE_NAME="\"jailhouse\"" \
+   $(NULL)
+virtjailhoused_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS)
+virtjailhoused_LDADD = $(REMOTE_DAEMON_LD_ADD)
+
 endif WITH_JAILHOUSE
diff --git a/src/jailhouse/jailhouse.conf b/src/jailhouse/jailhouse.conf
new file mode 100644
index 00..587068b9d0
--- /dev/null
+++ b/src/jailhouse/jailhouse.conf
@@ -0,0 +1,10 @@
+# Configuration for the Jailhouse driver.
+
+# Jailhouse system configuration file to enable the Jailhouse hypervisor on the
+# system. This is a required configuration parameter for the driver.
+#system_config = "/etc/libvirt/jailhouse/qemu-x86.cell"
+
+# Specify a directory which contains all the non-root cell configurations which
+# should be created by the driver at the startup. This is optional. Default
+# value of "/etc/libvirt/jailhouse/cells/" will be used if left empty.
+#non_root_cells_dir = "/etc/libvirt/jailhouse/cells/"
diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
new file mode 100644
index 00..cda00b50e7
--- /dev/null
+++ b/src/jailhouse/jailhouse_api.c
@@ -0,0 +1,372 @@
+/*
+ * jailhouse_api.c: Jailhouse API
+ *
+ * Copyright (C) 2020 Prakhar Bansal
+ *
+ * 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 

[libvirt PATCH 5/9] Jailhouse driver: Implementation of DomainInfo/State/List

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 src/jailhouse/jailhouse_driver.c | 102 +--
 1 file changed, 83 insertions(+), 19 deletions(-)

diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
index 5b7bdc92d8..2bb249f996 100644
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -267,19 +267,26 @@ static int
 jailhouseNodeGetInfo(virConnectPtr conn,
  virNodeInfoPtr nodeinfo)
 {
-UNUSED(conn);
-UNUSED(nodeinfo);
-return -1;
+if (virNodeGetInfoEnsureACL(conn) < 0)
+return -1;
+
+return virCapabilitiesGetNodeInfo(nodeinfo);
 }
 
 static int
 jailhouseConnectListAllDomains(virConnectPtr conn,
-   virDomainPtr **domain, unsigned int flags)
+   virDomainPtr **domains,
+   unsigned int flags)
 {
-UNUSED(conn);
-UNUSED(domain);
-UNUSED(flags);
-return -1;
+virJailhouseDriverPtr driver = conn->privateData;
+
+virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
+
+if (virConnectListAllDomainsEnsureACL(conn) < 0)
+return -1;
+
+return virDomainObjListExport(driver->domains, conn, domains,
+  virConnectListAllDomainsCheckACL, flags);
 }
 
 static virDomainPtr
@@ -522,30 +529,87 @@ jailhouseDomainDestroy(virDomainPtr domain)
 }
 
 static int
-jailhouseDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
+virjailhouseGetDomainTotalCpuStats(virDomainObjPtr cell,
+   unsigned long long *cpustats)
 {
-UNUSED(domain);
-UNUSED(info);
+// TODO(Prakhar): Not implemented yet.
+UNUSED(cell);
+UNUSED(cpustats);
 return -1;
 }
 
+static int
+jailhouseDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
+{
+virDomainObjPtr cell;
+int ret = -1;
+
+if (!(cell = virJailhouseDomObjFromDomain(domain)))
+goto cleanup;
+
+if (virDomainGetInfoEnsureACL(domain->conn, cell->def) < 0)
+goto cleanup;
+
+if (virDomainObjIsActive(cell)) {
+if (virjailhouseGetDomainTotalCpuStats(cell, &(info->cpuTime)) < 0)
+goto cleanup;
+} else {
+info->cpuTime = 0;
+}
+
+info->state = virDomainObjGetState(cell, NULL);
+info->maxMem = virDomainDefGetMemoryTotal(cell->def);
+info->nrVirtCpu = virDomainDefGetVcpus(cell->def);
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
 static int
 jailhouseDomainGetState(virDomainPtr domain,
 int *state, int *reason, unsigned int flags)
 {
-UNUSED(domain);
-UNUSED(state);
-UNUSED(reason);
-UNUSED(flags);
-return -1;
+virDomainObjPtr cell;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(cell = virJailhouseDomObjFromDomain(domain)))
+goto cleanup;
+
+if (virDomainGetStateEnsureACL(domain->conn, cell->def) < 0)
+   goto cleanup;
+
+*state = virDomainObjGetState(cell, reason);
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
 }
 
 static char *
 jailhouseDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
 {
-UNUSED(domain);
-UNUSED(flags);
-return NULL;
+virDomainObjPtr cell;
+char *ret = NULL;
+
+virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
+
+if (!(cell = virJailhouseDomObjFromDomain(domain)))
+goto cleanup;
+
+if (virDomainGetXMLDescEnsureACL(domain->conn, cell->def, flags) < 0)
+goto cleanup;
+
+ret = virDomainDefFormat(cell->def, NULL /* xmlopt */,
+ virDomainDefFormatConvertXMLFlags(flags));
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
 }
 
 static virHypervisorDriver jailhouseHypervisorDriver = {
-- 
2.26.2

[libvirt PATCH 3/9] Jailhouse driver: Implementation of ConnectGetType

[Daniel P . Berrangé]

From: Prakhar Bansal 

---
 src/jailhouse/jailhouse_driver.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
index ac9da4c85d..75bf41fc11 100644
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -32,8 +32,10 @@
 #include "viralloc.h"
 #include "virfile.h"
 #include "virlog.h"
+#include "virutil.h"
 #include "vircommand.h"
 #include "virpidfile.h"
+#include "access/viraccessapicheck.h"
 
 #define VIR_FROM_THIS VIR_FROM_JAILHOUSE
 
@@ -241,16 +243,19 @@ jailhouseStateInitialize(bool privileged G_GNUC_UNUSED,
 static const char *
 jailhouseConnectGetType(virConnectPtr conn)
 {
-UNUSED(conn);
-return NULL;
+if (virConnectGetTypeEnsureACL(conn) < 0)
+return NULL;
 
+return "JAILHOUSE";
 }
 
 static char *
 jailhouseConnectGetHostname(virConnectPtr conn)
 {
-UNUSED(conn);
-return NULL;
+if (virConnectGetHostnameEnsureACL(conn) < 0)
+return NULL;
+
+return virGetHostname();
 }
 
 static int
@@ -263,7 +268,7 @@ jailhouseNodeGetInfo(virConnectPtr conn, virNodeInfoPtr 
info)
 
 static int
 jailhouseConnectListAllDomains(virConnectPtr conn,
-   virDomainPtr ** domain, unsigned int flags)
+   virDomainPtr **domain, unsigned int flags)
 {
 UNUSED(conn);
 UNUSED(domain);
@@ -300,7 +305,6 @@ jailhouseDomainCreate(virDomainPtr domain)
 {
 UNUSED(domain);
 return -1;
-
 }
 
 static int
@@ -350,18 +354,18 @@ static virHypervisorDriver jailhouseHypervisorDriver = {
 .connectOpen = jailhouseConnectOpen,/* 6.3.0 */
 .connectClose = jailhouseConnectClose,  /* 6.3.0 */
 .connectListAllDomains = jailhouseConnectListAllDomains,/* 6.3.0 */
-.domainLookupByID = jailhouseDomainLookupByID,  /* 6.3.0 */
-.domainLookupByUUID = jailhouseDomainLookupByUUID,  /* 6.3.0 */
-.domainLookupByName = jailhouseDomainLookupByName,  /* 6.3.0 */
-.domainGetXMLDesc = jailhouseDomainGetXMLDesc,  /* 6.3.0 */
-.domainCreate = jailhouseDomainCreate,  /* 6.3.0 */
 .connectGetType = jailhouseConnectGetType,  /* 6.3.0 */
 .connectGetHostname = jailhouseConnectGetHostname,  /* 6.3.0 */
-.nodeGetInfo = jailhouseNodeGetInfo,/* 6.3.0 */
+.domainCreate = jailhouseDomainCreate,  /* 6.3.0 */
 .domainShutdown = jailhouseDomainShutdown,  /* 6.3.0 */
 .domainDestroy = jailhouseDomainDestroy,/* 6.3.0 */
 .domainGetInfo = jailhouseDomainGetInfo,/* 6.3.0 */
 .domainGetState = jailhouseDomainGetState,  /* 6.3.0 */
+.domainLookupByID = jailhouseDomainLookupByID,  /* 6.3.0 */
+.domainLookupByUUID = jailhouseDomainLookupByUUID,  /* 6.3.0 */
+.domainLookupByName = jailhouseDomainLookupByName,  /* 6.3.0 */
+.domainGetXMLDesc = jailhouseDomainGetXMLDesc,  /* 6.3.0 */
+.nodeGetInfo = jailhouseNodeGetInfo,/* 6.3.0 */
 };
 
 
-- 
2.26.2

[libvirt PATCH 0/9] [GSoC]: Libvirt driver for Jailhouse

[Daniel P . Berrangé]

I'm sending this series on behalf of Prakhar Bansal who has been unable
to sucessfully send the whole series. GMail is blocking sending of
certain patches for unknown reasons.

  https://www.redhat.com/archives/libvir-list/2020-September/msg00309.html

It is also available at

  https://github.com/pbansal2/libvirt/tree/jailhouse_driver

Prakhar Bansal (9):
  Jailhouse driver: first commit with skeleton code
  Jailhouse driver: Implementation of ConnectOpen
  Jailhouse driver: Implementation of ConnectGetType
  Jailhouse driver: Implementation of DomainCreate* callbacks
  Jailhouse driver: Implementation of DomainInfo/State/List
  Jailhouse driver: Implementation of DomainLookup* callbacks
  Jailhouse driver: Implementation of DomainShutdown/Destroy callbacks
  Jailhouse driver: Fixes for creation of cells, fetching cell info,
disabling jailhouse hypervisor
  Jailhouse driver: Add events to events queue in
DomainCreate/Shutdown/Destroy

 include/libvirt/virterror.h  |   1 +
 libvirt.spec.in  |   7 +
 m4/virt-driver-jailhouse.m4  |  42 ++
 meson.build  |   4 +
 meson_options.txt|   1 +
 po/POTFILES.in   |   2 +
 src/conf/domain_conf.c   |   1 +
 src/conf/domain_conf.h   |   1 +
 src/jailhouse/Makefile.inc.am|  47 ++
 src/jailhouse/jailhouse.conf |  10 +
 src/jailhouse/jailhouse_api.c| 458 +++
 src/jailhouse/jailhouse_api.h| 103 
 src/jailhouse/jailhouse_driver.c | 837 +++
 src/jailhouse/jailhouse_driver.h |  82 +++
 src/jailhouse/libvirtd_jailhouse.aug |  43 ++
 src/jailhouse/meson.build|  49 ++
 src/meson.build  |   1 +
 src/qemu/qemu_command.c  |   1 +
 src/remote/remote_daemon.c   |   4 +
 src/remote/remote_daemon_dispatch.c  |   3 +-
 src/util/virerror.c  |   1 +
 21 files changed, 1697 insertions(+), 1 deletion(-)
 create mode 100644 m4/virt-driver-jailhouse.m4
 create mode 100644 src/jailhouse/Makefile.inc.am
 create mode 100644 src/jailhouse/jailhouse.conf
 create mode 100644 src/jailhouse/jailhouse_api.c
 create mode 100644 src/jailhouse/jailhouse_api.h
 create mode 100644 src/jailhouse/jailhouse_driver.c
 create mode 100644 src/jailhouse/jailhouse_driver.h
 create mode 100644 src/jailhouse/libvirtd_jailhouse.aug
 create mode 100644 src/jailhouse/meson.build

-- 
2.26.2

Re: [libvirt PATCH 01/14] remove HAL node device driver

[Pavel Hrdina]

On Thu, Sep 17, 2020 at 04:30:57PM +0200, Andrea Bolognani wrote:
> On Thu, 2020-09-17 at 15:01 +0200, Michal Privoznik wrote:
> > And also it would be nice to mention this in the news file.
> 
> ... in addition to the switch to GLib's DBus client implementation,
> which removes the libdbus dependency and is thus very relevant to
> packagers.

Thanks, I'll fix the missing removals and post news as separate patch.

Pavel


signature.asc
Description: PGP signature

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

[Daniel P . Berrangé]

On Thu, Sep 17, 2020 at 06:55:36PM +0400, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote:
> > >   Daniel P. Berrangé wrote:
> > > 
> > > > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> > > > >   Daniel P. Berrangé wrote:
> > > > > 
> > > > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > > > > > > index fc52280361..52a055f205 100644
> > > > > > > --- a/src/bhyve/bhyve_device.c
> > > > > > > +++ b/src/bhyve/bhyve_device.c
> > > > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def 
> > > > > > > G_GNUC_UNUSED,
> > > > > > >  if (addr->slot == 0) {
> > > > > > >  return 0;
> > > > > > >  } else if (addr->slot == 1) {
> > > > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > > -   _("PCI bus 0 slot 1 is reserved for 
> > > > > > > the implicit "
> > > > > > > - "LPC PCI-ISA bridge"));
> > > > > > > -return -1;
> > > > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> > > > > > > +  device->data.controller->type == 
> > > > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> > > > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > > +_("PCI bus 0 slot 1 is reserved 
> > > > > > > for the implicit "
> > > > > > > +  "LPC PCI-ISA bridge"));
> > > > > > > + return -1;
> > > > > > > +} else {
> > > > > > > +/* We reserve slot 1 for LPC in 
> > > > > > > bhyveAssignDevicePCISlots(), so exit early */
> > > > > > > +return 0;
> > > > > > > +}
> > > > > > 
> > > > > > I forgot to respond to your followup comments on v4
> > > > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> > > > > > 
> > > > > > > > 
> > > > > > > > IIUC, this series makes it possible to put the TPC in a 
> > > > > > > > different
> > > > > > > > slot, so does it still make sense to forbid use of slot 1 as a
> > > > > > > > hardcoded rule ?
> > > > > > >
> > > > > > > IIRC, the idea behind that is to give some time window for users 
> > > > > > > to
> > > > > > > allow moving guests from the new version to the old one. If we 
> > > > > > > allow to
> > > > > > > use slot 1, it won't be possible to move the guest to the old 
> > > > > > > libvirt as
> > > > > > > it will complain slot 1 should be used only for LPC.
> > > > > > 
> > > > > > If the user has decided to move their LPC to a slot != 1, then it is
> > > > > > already impossible to migrate the guest to an old libvirt.
> > > > > > 
> > > > > > If the user wants to explicitly specify another device in slot 1, 
> > > > > > then
> > > > > > we should not prevent that.
> > > > > > 
> > > > > > We just need to make sure that if no LPC is in the XML, and no other
> > > > > > device explicitly has slot 1, then make sure to auto-assign LPC in 
> > > > > > slot 1
> > > > > > not some other device.
> > > > > 
> > > > > I've started playing with that and remembered some more corner cases.
> > > > > 
> > > > > To elaborate on your example, i.e. "no explicit LPC in XML AND no 
> > > > > other
> > > > > device explicitly on slot 1": these conditions are not specific 
> > > > > enough to
> > > > > tell whether an LPC device will be added or not.
> > > > > 
> > > > > In case if the LPC device was not explicitly specified by a user,
> > > > > the bhyve driver tries to add it if it's needed (it's required
> > > > > for serial console, bootloader, and video devices;
> > > > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start
> > > > > without the LPC device.
> > > > > 
> > > > > This could lead to a case when a user starts a domain in configuration
> > > > > that does not require LPC device, but has e.g. a network device on
> > > > > a PCI controller that's auto-assigned to slot 1.
> > > > > 
> > > > > Later user decides to change the configuration and adds a video 
> > > > > device,
> > > > > which requires LPC. This will lead to addresses changes, as LPC will 
> > > > > go
> > > > > to slot 1 and a network device's controller will go from slot 1 to 
> > > > > slot
> > > > > 2, which could be troublesome in some guest OSes.
> > > > 
> > > > First of all, lets me clear that we're talking about persistent guests
> > > > here, not transient guests.
> > > > 
> > > > With a persistent guest, the PCI addresses are assigned at the time
> > > > the XML arrives in virDomainDefineXML. If nothing requires the LPC
> > > > at this time, then a NIC could get given slot 1. This is recorded in
> > > > the persistent XML.
> > > > 
> > > > If the user later uses 'virsh edit' to modify the XML and add a video
> > > > device, libvirt will see that the 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

[Roman Bogorodskiy]

  Daniel P. Berrangé wrote:

> On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote:
> >   Daniel P. Berrangé wrote:
> > 
> > > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> > > >   Daniel P. Berrangé wrote:
> > > > 
> > > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > > > > > index fc52280361..52a055f205 100644
> > > > > > --- a/src/bhyve/bhyve_device.c
> > > > > > +++ b/src/bhyve/bhyve_device.c
> > > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def 
> > > > > > G_GNUC_UNUSED,
> > > > > >  if (addr->slot == 0) {
> > > > > >  return 0;
> > > > > >  } else if (addr->slot == 1) {
> > > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > -   _("PCI bus 0 slot 1 is reserved for the 
> > > > > > implicit "
> > > > > > - "LPC PCI-ISA bridge"));
> > > > > > -return -1;
> > > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> > > > > > +  device->data.controller->type == 
> > > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> > > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > +_("PCI bus 0 slot 1 is reserved 
> > > > > > for the implicit "
> > > > > > +  "LPC PCI-ISA bridge"));
> > > > > > + return -1;
> > > > > > +} else {
> > > > > > +/* We reserve slot 1 for LPC in 
> > > > > > bhyveAssignDevicePCISlots(), so exit early */
> > > > > > +return 0;
> > > > > > +}
> > > > > 
> > > > > I forgot to respond to your followup comments on v4
> > > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> > > > > 
> > > > > > > 
> > > > > > > IIUC, this series makes it possible to put the TPC in a different
> > > > > > > slot, so does it still make sense to forbid use of slot 1 as a
> > > > > > > hardcoded rule ?
> > > > > >
> > > > > > IIRC, the idea behind that is to give some time window for users to
> > > > > > allow moving guests from the new version to the old one. If we 
> > > > > > allow to
> > > > > > use slot 1, it won't be possible to move the guest to the old 
> > > > > > libvirt as
> > > > > > it will complain slot 1 should be used only for LPC.
> > > > > 
> > > > > If the user has decided to move their LPC to a slot != 1, then it is
> > > > > already impossible to migrate the guest to an old libvirt.
> > > > > 
> > > > > If the user wants to explicitly specify another device in slot 1, then
> > > > > we should not prevent that.
> > > > > 
> > > > > We just need to make sure that if no LPC is in the XML, and no other
> > > > > device explicitly has slot 1, then make sure to auto-assign LPC in 
> > > > > slot 1
> > > > > not some other device.
> > > > 
> > > > I've started playing with that and remembered some more corner cases.
> > > > 
> > > > To elaborate on your example, i.e. "no explicit LPC in XML AND no other
> > > > device explicitly on slot 1": these conditions are not specific enough 
> > > > to
> > > > tell whether an LPC device will be added or not.
> > > > 
> > > > In case if the LPC device was not explicitly specified by a user,
> > > > the bhyve driver tries to add it if it's needed (it's required
> > > > for serial console, bootloader, and video devices;
> > > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start
> > > > without the LPC device.
> > > > 
> > > > This could lead to a case when a user starts a domain in configuration
> > > > that does not require LPC device, but has e.g. a network device on
> > > > a PCI controller that's auto-assigned to slot 1.
> > > > 
> > > > Later user decides to change the configuration and adds a video device,
> > > > which requires LPC. This will lead to addresses changes, as LPC will go
> > > > to slot 1 and a network device's controller will go from slot 1 to slot
> > > > 2, which could be troublesome in some guest OSes.
> > > 
> > > First of all, lets me clear that we're talking about persistent guests
> > > here, not transient guests.
> > > 
> > > With a persistent guest, the PCI addresses are assigned at the time
> > > the XML arrives in virDomainDefineXML. If nothing requires the LPC
> > > at this time, then a NIC could get given slot 1. This is recorded in
> > > the persistent XML.
> > > 
> > > If the user later uses 'virsh edit' to modify the XML and add a video
> > > device, libvirt will see that the NIC is already on slot 1. It will
> > > thus have to give the LPC slot 2 (or whatever is free). The NIC will
> > > not move from slot 1, as that slot is considered taken at this time.
> > > 
> > > The same is true if using virDomainAttachDevice to add a video
> > > card. Any modifications to the XML must never change addresses that
> > > are 

Re: [libvirt PATCH 01/14] remove HAL node device driver

[Andrea Bolognani]

On Thu, 2020-09-17 at 15:01 +0200, Michal Privoznik wrote:
> And also it would be nice to mention this in the news file.

... in addition to the switch to GLib's DBus client implementation,
which removes the libdbus dependency and is thus very relevant to
packagers.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

[Daniel P . Berrangé]

On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> > >   Daniel P. Berrangé wrote:
> > > 
> > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > > > > index fc52280361..52a055f205 100644
> > > > > --- a/src/bhyve/bhyve_device.c
> > > > > +++ b/src/bhyve/bhyve_device.c
> > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def 
> > > > > G_GNUC_UNUSED,
> > > > >  if (addr->slot == 0) {
> > > > >  return 0;
> > > > >  } else if (addr->slot == 1) {
> > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > -   _("PCI bus 0 slot 1 is reserved for the 
> > > > > implicit "
> > > > > - "LPC PCI-ISA bridge"));
> > > > > -return -1;
> > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> > > > > +  device->data.controller->type == 
> > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > +_("PCI bus 0 slot 1 is reserved for 
> > > > > the implicit "
> > > > > +  "LPC PCI-ISA bridge"));
> > > > > + return -1;
> > > > > +} else {
> > > > > +/* We reserve slot 1 for LPC in 
> > > > > bhyveAssignDevicePCISlots(), so exit early */
> > > > > +return 0;
> > > > > +}
> > > > 
> > > > I forgot to respond to your followup comments on v4
> > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> > > > 
> > > > > > 
> > > > > > IIUC, this series makes it possible to put the TPC in a different
> > > > > > slot, so does it still make sense to forbid use of slot 1 as a
> > > > > > hardcoded rule ?
> > > > >
> > > > > IIRC, the idea behind that is to give some time window for users to
> > > > > allow moving guests from the new version to the old one. If we allow 
> > > > > to
> > > > > use slot 1, it won't be possible to move the guest to the old libvirt 
> > > > > as
> > > > > it will complain slot 1 should be used only for LPC.
> > > > 
> > > > If the user has decided to move their LPC to a slot != 1, then it is
> > > > already impossible to migrate the guest to an old libvirt.
> > > > 
> > > > If the user wants to explicitly specify another device in slot 1, then
> > > > we should not prevent that.
> > > > 
> > > > We just need to make sure that if no LPC is in the XML, and no other
> > > > device explicitly has slot 1, then make sure to auto-assign LPC in slot 
> > > > 1
> > > > not some other device.
> > > 
> > > I've started playing with that and remembered some more corner cases.
> > > 
> > > To elaborate on your example, i.e. "no explicit LPC in XML AND no other
> > > device explicitly on slot 1": these conditions are not specific enough to
> > > tell whether an LPC device will be added or not.
> > > 
> > > In case if the LPC device was not explicitly specified by a user,
> > > the bhyve driver tries to add it if it's needed (it's required
> > > for serial console, bootloader, and video devices;
> > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start
> > > without the LPC device.
> > > 
> > > This could lead to a case when a user starts a domain in configuration
> > > that does not require LPC device, but has e.g. a network device on
> > > a PCI controller that's auto-assigned to slot 1.
> > > 
> > > Later user decides to change the configuration and adds a video device,
> > > which requires LPC. This will lead to addresses changes, as LPC will go
> > > to slot 1 and a network device's controller will go from slot 1 to slot
> > > 2, which could be troublesome in some guest OSes.
> > 
> > First of all, lets me clear that we're talking about persistent guests
> > here, not transient guests.
> > 
> > With a persistent guest, the PCI addresses are assigned at the time
> > the XML arrives in virDomainDefineXML. If nothing requires the LPC
> > at this time, then a NIC could get given slot 1. This is recorded in
> > the persistent XML.
> > 
> > If the user later uses 'virsh edit' to modify the XML and add a video
> > device, libvirt will see that the NIC is already on slot 1. It will
> > thus have to give the LPC slot 2 (or whatever is free). The NIC will
> > not move from slot 1, as that slot is considered taken at this time.
> > 
> > The same is true if using virDomainAttachDevice to add a video
> > card. Any modifications to the XML must never change addresses that
> > are currently recorded in the XML, only ever place devices in new
> > unused slots.
> 
> Sorry, I should have stated that I was assuming that LPC always
> gets slot 1 assigned if it has no address explicitly assigned by the user
> in the 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

[Roman Bogorodskiy]

  Daniel P. Berrangé wrote:

> On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> >   Daniel P. Berrangé wrote:
> > 
> > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > Support modeling of the 'isa' controller for bhyve. User can manually
> > > > define any PCI slot for the 'isa' controller, including PCI slot 1,
> > > > but other devices are not allowed to use this address.
> > > > 
> > > > When domain configuration requires the 'isa' controller to be present,
> > > > automatically add it on domain post-parse stage.
> > > > 
> > > > Now, as this controller is always available when needed, it's not
> > > > necessary to implicitly add it to the bhyve command line, so remove
> > > > bhyveBuildLPCArgStr().
> > > > 
> > > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer
> > > > used outside of bhyve_domain.c.
> > > > 
> > > > As more than one ISA controller is not supported by bhyve,
> > > > and multiple controllers with the same index are forbidden,
> > > > so forbid ISA controllers with non-zero index for bhyve.
> > > > 
> > > > Signed-off-by: Roman Bogorodskiy 
> > > > ---
> > > >  src/bhyve/bhyve_command.c | 27 +++---
> > > >  src/bhyve/bhyve_device.c  | 23 +---
> > > >  src/bhyve/bhyve_domain.c  | 25 +++--
> > > >  src/bhyve/bhyve_domain.h  |  2 --
> > > >  ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
> > > >  ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
> > > >  ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
> > > >  ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
> > > >  ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
> > > >  ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
> > > >  ...argv-addr-non-isa-controller-on-slot-1.xml | 23 
> > > >  .../bhyvexml2argv-console.args|  2 +-
> > > >  .../bhyvexml2argv-isa-controller.args | 10 ++
> > > >  .../bhyvexml2argv-isa-controller.ldargs   |  3 ++
> > > >  .../bhyvexml2argv-isa-controller.xml  | 24 +
> > > >  ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
> > > >  .../bhyvexml2argv-serial-grub-nocons.args |  2 +-
> > > >  .../bhyvexml2argv-serial-grub.args|  2 +-
> > > >  .../bhyvexml2argv-serial.args |  2 +-
> > > >  .../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
> > > >  .../bhyvexml2argv-vnc-autoport.args   |  4 +--
> > > >  .../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
> > > >  .../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
> > > >  .../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
> > > >  .../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
> > > >  tests/bhyvexml2argvtest.c |  5 +++
> > > >  ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
> > > >  ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
> > > >  .../bhyvexml2xmlout-console.xml   |  3 ++
> > > >  .../bhyvexml2xmlout-isa-controller.xml| 36 +++
> > > >  .../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
> > > >  .../bhyvexml2xmlout-serial-grub.xml   |  3 ++
> > > >  .../bhyvexml2xmlout-serial.xml|  3 ++
> > > >  .../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
> > > >  .../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
> > > >  .../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
> > > >  .../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
> > > >  .../bhyvexml2xmlout-vnc.xml   |  3 ++
> > > >  tests/bhyvexml2xmltest.c  |  3 ++
> > > >  39 files changed, 378 insertions(+), 37 deletions(-)
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> > > >  create mode 100644 
> > > > 

[PATCH v3 0/6] qemu: implementation of transient disk option

[Masayoshi Mizuma]

This patchset tries to implement transient option for qcow2 and raw
format disk.

It gets user available to set  to the domain xml file like as:


  
  
  
  


Any changes which the Guest does to the disk is dropped when the Guest
is shutdowned.

There are some limitations for transient disk option so far:

- Supported disk format is qcow2 and raw
- blockdev capability is required for qemu
- Following features are blocked with transient disk option
  - blockjobs 
  - Migration
  - Disk hotplug

Masayoshi Mizuma (6):
  qemu: Block blockjobs when transient disk option is enabled
  qemu: Block disk hotplug when transient disk option is enabled
  qemu: Block migration when transient disk option is enabled
  qemu: update the validation for transient disk option
  qemu: Introduce to handle transient disk
  qemu: Add transient disk handler to start and stop the guest

 src/qemu/qemu_domain.c|   7 +++
 src/qemu/qemu_hotplug.c   |   6 +++
 src/qemu/qemu_migration.c |  10 
 src/qemu/qemu_process.c   |  10 
 src/qemu/qemu_snapshot.c  | 103 +++---
 src/qemu/qemu_snapshot.h  |   5 ++
 src/qemu/qemu_validate.c  |  25 -
 src/util/virstoragefile.c |   2 +
 src/util/virstoragefile.h |   4 ++
 9 files changed, 164 insertions(+), 8 deletions(-)

-- 
2.27.0

[PATCH v3 5/6] qemu: Introduce to handle transient disk

[Masayoshi Mizuma]

From: Masayoshi Mizuma 

Here is the implementation of transient option for qcow2 and raw format
disk. This gets available  directive in domain xml file
like as:


  
  
  
  


When the qemu command line options are built, a new qcow2 image is
created with backing qcow2 by using blockdev-snapshot command.
The backing image is the qcow2 file which is set as .
The filename of the new qcow2 image is original-source-file.TRANSIENT.

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_snapshot.c  | 103 +++---
 src/qemu/qemu_snapshot.h  |   5 ++
 src/util/virstoragefile.c |   2 +
 src/util/virstoragefile.h |   4 ++
 4 files changed, 107 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 1e8ea80b22..67fdc488e0 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1158,7 +1158,8 @@ qemuSnapshotCreateDiskActive(virQEMUDriverPtr driver,
  virHashTablePtr blockNamedNodeData,
  unsigned int flags,
  virQEMUDriverConfigPtr cfg,
- qemuDomainAsyncJob asyncJob)
+ qemuDomainAsyncJob asyncJob,
+ bool domSave)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(virJSONValue) actions = NULL;
@@ -1201,17 +1202,26 @@ qemuSnapshotCreateDiskActive(virQEMUDriverPtr driver,
 
 virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0);
 
-if (rc == 0)
+if (rc == 0) {
 qemuSnapshotDiskUpdateSource(driver, vm, dd, blockdev);
+
+if (dd->disk->transient) {
+/* Mark the transient working is completed to make sure we can 
*/
+/* remove the transient disk when the guest is shutdown. */
+dd->disk->src->transientEstablished = true;
+}
+}
 }
 
 if (rc < 0)
 goto cleanup;
 
-if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 ||
-(vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt,
-cfg->configDir) < 0))
-goto cleanup;
+if (domSave) {
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 ||
+(vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt,
+cfg->configDir) < 0))
+goto cleanup;
+}
 
 ret = 0;
 
@@ -1349,7 +1359,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
 
 if ((ret = qemuSnapshotCreateDiskActive(driver, vm, snap,
 blockNamedNodeData, flags, cfg,
-QEMU_ASYNC_JOB_SNAPSHOT)) < 0)
+QEMU_ASYNC_JOB_SNAPSHOT, true)) < 
0)
 goto cleanup;
 
 /* the snapshot is complete now */
@@ -2264,3 +2274,82 @@ qemuSnapshotDelete(virDomainObjPtr vm,
  cleanup:
 return ret;
 }
+
+
+static int
+qemuSnapshotSetupTransientDisk(virDomainSnapshotDiskDefPtr def,
+   virStorageSourcePtr src)
+{
+g_autoptr(virStorageSource) trans = NULL;
+
+if (!(trans = virStorageSourceNew()))
+return -1;
+
+trans->path = g_strdup_printf("%s.TRANSIENT", src->path);
+if (virFileExists(trans->path)) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("Transient disk '%s' for '%s' exists"),
+   trans->path, src->path);
+return -1;
+}
+
+trans->type = VIR_STORAGE_TYPE_FILE;
+trans->format = VIR_STORAGE_FILE_QCOW2;
+
+def->src = g_steal_pointer();
+
+return 0;
+}
+
+
+int
+qemuSnapshotCreateTransientDisk(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+int asyncJob)
+{
+int rc;
+size_t i;
+virDomainMomentObjPtr snap = NULL;
+g_autoptr(virDomainSnapshotDef) snapdef = NULL;
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
+
+if (!(snapdef = virDomainSnapshotDefNew()))
+return -1;
+
+snapdef->parent.name = g_strdup_printf("transient");
+
+snapdef->ndisks = vm->def->ndisks;
+if (VIR_ALLOC_N(snapdef->disks, snapdef->ndisks) < 0)
+return -1;
+
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+virDomainSnapshotDiskDefPtr snapdisk = &(snapdef->disks[i]);
+
+if (disk->transient) {
+if ((rc = qemuSnapshotSetupTransientDisk(snapdisk, disk->src)) < 0)
+return -1;
+
+} else {
+snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+}
+}
+
+if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, snapdef)))
+return 

[PATCH v3 2/6] qemu: Block disk hotplug when transient disk option is enabled

[Masayoshi Mizuma]

From: Masayoshi Mizuma 

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_hotplug.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e2c6e14c2e..20dcec7b65 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1031,6 +1031,12 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr 
driver,
 return -1;
 }
 
+if (disk->transient) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("transient disk hotplug isn't supported"));
+return -1;
+}
+
 if (virDomainDiskTranslateSourcePool(disk) < 0)
 goto cleanup;
 
-- 
2.27.0

[PATCH v3 6/6] qemu: Add transient disk handler to start and stop the guest

[Masayoshi Mizuma]

From: Masayoshi Mizuma 

The transient disk is attached before the guest starts.
Remove the transient disk when the guest does shutdown.

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_process.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1af35b933..387b8071f4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -60,6 +60,7 @@
 #include "qemu_firmware.h"
 #include "qemu_backup.h"
 #include "qemu_dbus.h"
+#include "qemu_snapshot.h"
 
 #include "cpu/cpu.h"
 #include "cpu/cpu_x86.h"
@@ -7053,6 +7054,10 @@ qemuProcessLaunch(virConnectPtr conn,
 qemuProcessAutoDestroyAdd(driver, vm, conn) < 0)
 goto cleanup;
 
+VIR_DEBUG("Setting up transient disk");
+if (qemuSnapshotCreateTransientDisk(driver, vm, asyncJob) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
@@ -7689,6 +7694,11 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 }
 
 qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src);
+
+if ((disk->transient) && (disk->src->transientEstablished)) {
+VIR_DEBUG("unlink transient disk: %s", disk->src->path);
+unlink(disk->src->path);
+}
 }
 }
 
-- 
2.27.0

[PATCH v3 4/6] qemu: update the validation for transient disk option

[Masayoshi Mizuma]

From: Masayoshi Mizuma 

Update validation of transient disk option. The option for qemu is supported
with under condistions.

- qemu has blockdev feature 
- the type is file and the format is qcow2 and raw
- writable disk

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_validate.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 070f1c962b..9cf78ca0c9 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -2002,6 +2002,28 @@ qemuValidateDomainDeviceDefDiskSerial(const char *value)
 }
 
 
+static int
+qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk,
+ virQEMUCapsPtr qemuCaps)
+{
+if ((!qemuCaps) || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV))
+return false;
+
+if (disk->src->readonly)
+return false;
+
+if ((disk->src->format != VIR_STORAGE_FILE_QCOW2) &&
+(disk->src->format != VIR_STORAGE_FILE_RAW) &&
+(disk->src->type != VIR_STORAGE_TYPE_FILE)) {
+return false;
+}
+
+if (virStorageSourceIsEmpty(disk->src))
+return false;
+
+return true;
+}
+
 static int
 qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
 virQEMUCapsPtr qemuCaps)
@@ -2186,7 +2208,8 @@ qemuValidateDomainDeviceDefDiskFrontend(const 
virDomainDiskDef *disk,
 }
 }
 
-if (disk->transient) {
+if ((disk->transient) &&
+!qemuValidateDomainDeviceDefDiskTransient(disk, qemuCaps)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("transient disks not supported yet"));
 return -1;
-- 
2.27.0

[PATCH v3 1/6] qemu: Block blockjobs when transient disk option is enabled

[Masayoshi Mizuma]

From: Masayoshi Mizuma 

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_domain.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 785cee6f18..1ce0f70971 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10794,6 +10794,13 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr vm,
 return false;
 }
 
+if (disk->transient) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _("block jobs are not supported on transient disk 
'%s'"),
+   disk->dst);
+return false;
+}
+
 return true;
 }
 
-- 
2.27.0

[PATCH v3 3/6] qemu: Block migration when transient disk option is enabled

[Masayoshi Mizuma]

From: Masayoshi Mizuma 

Block migration when transient disk option is enabled because migration
requires some blockjobs.

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_migration.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a530c17582..7316d74677 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1397,6 +1397,16 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
_("cannot migrate this domain without dbus-vmstate 
support"));
 return false;
 }
+
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+
+if (disk->transient) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("migration with transient disk is not 
supported"));
+return false;
+}
+}
 }
 
 return true;
-- 
2.27.0

Re: [libvirt PATCH 0/3] cpu: Make unknown XML elements fail CPU comparison

[Tim Wiederhake]

On Wed, 2020-09-16 at 14:48 +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 16, 2020 at 03:33:53PM +0200, Tim Wiederhake wrote:
> > We currently ignore unknown elements in the CPU XML description,
> > e.g. with vi=
> > rsh
> > cpu-compare and hypervisor-cpu-compare. This makes ' > name=3D"...=
> > "/>'
> > (note the typo in "faeture") semantically identic to ''. No
> > error is re=
> > ported.
> 
> This is an intentional implementation choice in libvirt. The XML
> documents
> that we're parsing are way to large and complex for us to justify
> adding
> code to report errors for every invalid attribute or child element
> name.
>
> We provide an RNG schema, however, which declares the valid set of
> attrs
> and thus by running RNG schema validation, a client can get a error
> report
> of invalid info.
> 
> We enable this validation by default in "virsh edit" and a few other
> similar commands.
> 
> There are certainly libvirt APIs though which accept XML and don't
> have
> a flag defined to enable RNG schema validation. I presume the CPU
> APIs
> are one such case.  We should add support for validation to all such
> APIs accepting XML documents, and wire it up in virsh.
> 
> Regards,
> Daniel

Hi Daniel, all,

thanks for the input!

My first attempt at the problem I described did in fact employ RNG
schema validation on the xml documents describing the CPU. The problem
though with that approach is that `virsh [hypervisor-]cpu-compare` can
consume quite different xml documents. The tests add more possible
document types, e.g. multicpu, which force the schema for a validation
performed in `virCPUDefParseXML` to basically import all other schemas
(which produced some naming conflicts that I was unable to solve) and,
secondly, perform a lot of unneccessary validation.

I created a schema that would perform only a check for "unknown"
elements and attributes in the cpu element and ignore everything else.
I attached this for your entertainment. There you can see what I meant
with "different xml documents".

Copying / moving the node into a new document just for validation seems
not like an elegant solution (if possible at all) and I am unsure about
potential performance impact.

The best solution in my opinion would be to not perform XPath
evaluation in the parser, but instead use some stream parser and error
out if the parser encounteres an element with an unknown name.

The next best solution in my opinion is the one you reviewed. I agree
with all comments made that this is far from perfect, but at least it
would trigger visible test fails when / if new elements and attributes
are added without that list being expanded.

I will further explore whether it is possible to do node-only
validation without creating an include-everything schema, but I would
be grateful for any hint or suggestion in that direction. We should
have at least some error reporting in the case I described initially.

Thanks,
Tim


only_cpu.rng
Description: XML document

Re: [libvirt PATCH 00/14] rewrite DBus to use GLib instead of libdbus

[Michal Privoznik]

On 9/17/20 10:29 AM, Pavel Hrdina wrote:

Pavel Hrdina (14):
   remove HAL node device driver
   tests: mock libdbus in networkxml2firewalltest
   util: introduce helpers for GLib DBus implementation
   tests/virmock: extend number of arguments
   tests: introduce virgdbusmock to mock GLib DBus functions
   src/util/virpolkit: convert to use GLib DBus
   src/util/virfirewalld: convert to use GLib DBus
   src/util/virsystemd: convert to use GLib DBus
   src/lxc/lxc_controller: convert to use GLib DBus
   src/network/bridge_driver: convert to use GLib DBus
   src/nwfilter/nwfilter_driver: convert to use GLib DBus
   src/remote/remote_daemon: convert to use GLib DBus
   src/rpc/virnetdaemon: convert to use GLib DBus
   drop libdbus from libvirt

  libvirt.spec.in  |3 -
  meson.build  |   42 +-
  meson_options.txt|2 -
  po/POTFILES.in   |3 +-
  src/libvirt_private.syms |   31 +-
  src/libvirt_probes.d |6 -
  src/lxc/lxc_controller.c |6 +-
  src/lxc/meson.build  |1 -
  src/meson.build  |1 -
  src/network/bridge_driver.c  |   81 +-
  src/network/meson.build  |2 -
  src/node_device/meson.build  |5 -
  src/node_device/node_device_driver.c |   10 +-
  src/node_device/node_device_driver.h |5 -
  src/node_device/node_device_hal.c|  843 
  src/node_device/node_device_hal.h|   22 -
  src/nwfilter/meson.build |1 -
  src/nwfilter/nwfilter_driver.c   |  127 +-
  src/remote/remote_daemon.c   |   73 +-
  src/remote/remote_daemon_dispatch.c  |1 -
  src/rpc/meson.build  |1 -
  src/rpc/virnetdaemon.c   |  115 +-
  src/util/meson.build |3 +-
  src/util/virdbus.c   | 1871 --
  src/util/virdbus.h   |   76 --
  src/util/virdbuspriv.h   |   56 -
  src/util/virfirewalld.c  |  197 ++-
  src/util/virgdbus.c  |  425 ++
  src/util/virgdbus.h  |   79 ++
  src/util/virpolkit.c |  115 +-
  src/util/virsystemd.c|  371 ++---
  tests/meson.build|   29 +-
  tests/networkxml2firewalltest.c  |   30 +-
  tests/virdbusmock.c  |   62 -
  tests/virdbustest.c  |  728 --
  tests/virfirewalltest.c  |  154 +--
  tests/virgdbusmock.c |   85 ++
  tests/virmock.h  |   10 +-
  tests/virpolkittest.c|  112 +-
  tests/virsystemdtest.c   |  173 +--
  40 files changed, 1330 insertions(+), 4627 deletions(-)
  delete mode 100644 src/node_device/node_device_hal.c
  delete mode 100644 src/node_device/node_device_hal.h
  delete mode 100644 src/util/virdbus.c
  delete mode 100644 src/util/virdbus.h
  delete mode 100644 src/util/virdbuspriv.h
  create mode 100644 src/util/virgdbus.c
  create mode 100644 src/util/virgdbus.h
  delete mode 100644 tests/virdbusmock.c
  delete mode 100644 tests/virdbustest.c
  create mode 100644 tests/virgdbusmock.c



Reviewed-by: Michal Privoznik 

Michal

Re: [libvirt PATCH 01/14] remove HAL node device driver

[Michal Privoznik]

On 9/17/20 10:29 AM, Pavel Hrdina wrote:

There was one attempt a year ago done by me to drop HAL [1] but it was
never resolved. There was another time when Dan suggested to drop HAL
driver [2] but it was decided to keep it around in case device
assignment will be implemented for FreeBSD and the fact that
virt-manager uses node device driver [3].

I checked git history and code and it doesn't look like bhyve supports
device assignment so from that POV it should not block removing HAL.

The argument about virt-manager is not strong as well because libvirt
installed from FreeBSD packages doesn't have HAL support so it will not
affect these users as well [4].

The only users affected by this change would be the ones compiling
libvirt from GIT on FreeBSD.

I looked into alternatives and there is libudev-devd package on FreeBSD
but unfortunately it doesn't work as it doesn't list any devices when
used with libvirt. It provides libudev APIs using devd.

I also looked into devd directly and it provides some APIs but there are
no APIs for device monitoring and events so that would have to be
somehow done by libvirt.

Main motivation for dropping HAL support is to replace libdbus with GLib
dbus implementation and it cannot be done with HAL driver present in
libvirt because HAL APIs heavily depends on symbols provided by libdbus.

[1] 
[2] 
[3] 
[4] 

Signed-off-by: Pavel Hrdina 
---
  meson.build  |   9 +-
  meson_options.txt|   1 -
  po/POTFILES.in   |   1 -
  src/node_device/meson.build  |   5 -
  src/node_device/node_device_driver.c |  10 +-
  src/node_device/node_device_driver.h |   5 -
  src/node_device/node_device_hal.c| 843 ---
  src/node_device/node_device_hal.h|  22 -
  8 files changed, 3 insertions(+), 893 deletions(-)
  delete mode 100644 src/node_device/node_device_hal.c
  delete mode 100644 src/node_device/node_device_hal.h


Couple of missed occurrences:

diff --git i/docs/drvnodedev.html.in w/docs/drvnodedev.html.in
index ab87b6bbd2..0823c1888d 100644
--- i/docs/drvnodedev.html.in
+++ w/docs/drvnodedev.html.in
@@ -23,8 +23,7 @@
   (https://wiki.libvirt.org/page/NPIV_in_libvirt;>more 
info about NPIV)).
   Devices on the host system are arranged in a tree-like 
hierarchy, with
   the root node being called computer. The node 
device driver
-  supports two backends to manage the devices, HAL and udev, with 
the former

-  being deprecated in favour of the latter.
+  supports udev backend (HAL backend was removed in 
6.8.0).

 

 
diff --git i/libvirt.spec.in w/libvirt.spec.in
index 62b401bd08..d9bfed0bef 100644
--- i/libvirt.spec.in
+++ w/libvirt.spec.in
@@ -1166,7 +1166,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' 
%{_specdir}/%{name}.spec)

%{?arg_selinux_mount} \
-Dapparmor=disabled \
-Dsecdriver_apparmor=disabled \
-   -Dhal=disabled \
-Dudev=enabled \
-Dyajl=enabled \
%{?arg_sanlock} \
diff --git i/scripts/hvsupport.py w/scripts/hvsupport.py
index 14c41da348..e2aab8feff 100755
--- i/scripts/hvsupport.py
+++ w/scripts/hvsupport.py
@@ -60,7 +60,6 @@ for root, dirs, files in os.walk(os.path.join(srcdir, 
"src")):

 file.endswith("common.c") or
 file.endswith("tmpl.c") or
 file.endswith("monitor.c") or
-file.endswith("hal.c") or
 file.endswith("udev.c")):
 srcs.append(os.path.join(root, file))


And also it would be nice to mention this in the news file.

Michal

[libvirt PATCH v2 4/4] storage: add support for qcow2 LUKS encryption

[Daniel P . Berrangé]

The storage driver was wired up to support creating raw volumes in LUKS
format, but was never adapted to support LUKS-in-qcow2. This is trivial
as it merely requires the encryption properties to be prefixed with
the "encrypt." prefix, and "encrypt.format=luks" when creating the
volume.

Signed-off-by: Daniel P. Berrangé 
---
 src/storage/storage_util.c| 70 ++-
 src/util/virqemu.c| 23 --
 src/util/virqemu.h|  1 +
 .../qcow2-luks-convert-encrypt.argv   | 18 +
 .../qcow2-luks-convert-encrypt2fileqcow2.argv | 14 
 .../qcow2-luks-convert-encrypt2fileraw.argv   | 13 
 tests/storagevolxml2argvdata/qcow2-luks.argv  |  8 +++
 tests/storagevolxml2argvtest.c| 15 
 .../vol-qcow2-luks-convert.xml| 31 
 tests/storagevolxml2xmlin/vol-qcow2-luks.xml  | 31 
 tests/storagevolxml2xmlout/vol-qcow2-luks.xml | 31 
 tests/storagevolxml2xmltest.c |  1 +
 12 files changed, 234 insertions(+), 22 deletions(-)
 create mode 100644 tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv
 create mode 100644 
tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv
 create mode 100644 
tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileraw.argv
 create mode 100644 tests/storagevolxml2argvdata/qcow2-luks.argv
 create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-luks-convert.xml
 create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-luks.xml
 create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-luks.xml

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index cf82ea0a87..9171cb084f 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -707,7 +707,7 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo,
   
virStorageFileFormatTypeToString(info->backingFormat));
 
 if (encinfo)
-virQEMUBuildQemuImgKeySecretOpts(, encinfo, info->secretAlias);
+virQEMUBuildQemuImgKeySecretOpts(, info->format, encinfo, 
info->secretAlias);
 
 if (info->preallocate) {
 if (info->size_arg > info->allocation)
@@ -761,7 +761,8 @@ storageBackendCreateQemuImgCheckEncryption(int format,
 {
 virStorageEncryptionPtr enc = vol->target.encryption;
 
-if (format == VIR_STORAGE_FILE_RAW) {
+if (format == VIR_STORAGE_FILE_RAW ||
+format == VIR_STORAGE_FILE_QCOW2) {
 if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported volume encryption format %d"),
@@ -927,21 +928,34 @@ storageBackendCreateQemuImgSecretObject(virCommandPtr cmd,
 }
 
 
-/* Add a --image-opts to the qemu-img resize command line:
+/* Add a --image-opts to the qemu-img resize command line for use
+ * with encryption:
  *--image-opts driver=luks,file.filename=$volpath,key-secret=$secretAlias
+ * or
+ *--image-opts 
driver=qcow2,file.filename=$volpath,encrypt.key-secret=$secretAlias
  *
- *NB: format=raw is assumed
  */
 static int
 storageBackendResizeQemuImgImageOpts(virCommandPtr cmd,
+ int format,
  const char *path,
  const char *secretAlias)
 {
 g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 g_autofree char *commandStr = NULL;
+const char *encprefix;
+const char *driver;
 
-virBufferAsprintf(, "driver=luks,key-secret=%s,file.filename=",
-  secretAlias);
+if (format == VIR_STORAGE_FILE_QCOW2) {
+driver = "qcow2";
+encprefix = "encrypt.";
+} else {
+driver = "luks";
+encprefix = "";
+}
+
+virBufferAsprintf(, "driver=%s,%skey-secret=%s,file.filename=",
+  driver, encprefix, secretAlias);
 virQEMUBuildBufferEscapeComma(, path);
 
 commandStr = virBufferContentAndReset();
@@ -1006,6 +1020,16 @@ 
virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
 return -1;
 }
 }
+if (inputvol && inputvol->target.format == VIR_STORAGE_FILE_RAW &&
+inputvol->target.encryption) {
+if (inputvol->target.encryption->format == 
VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+info->inputType = "luks";
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Only luks encryption is supported for raw 
files"));
+return -1;
+}
+}
 
 if (inputvol &&
 storageBackendCreateQemuImgSetInput(inputvol, convertStep, info) < 0)
@@ -1056,6 +1080,8 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 virStorageEncryptionPtr inputenc = inputvol ? inputvol->target.encryption 
: NULL;
 virStorageEncryptionInfoDefPtr encinfo = NULL;
 g_autofree char *inputSecretAlias = NULL;
+  

[libvirt PATCH v2 2/4] util: detect LUKS encryption scheme in qcow2 files

[Daniel P . Berrangé]

Crypt method number 2 indicates LUKS format.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virstoragefile.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 97a346db28..42341150e5 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -290,6 +290,22 @@ static struct FileEncryptionInfo const 
qcow2EncryptionInfo[] = {
 
 .payloadOffset = -1,
 },
+{
+.format = VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
+
+.magicOffset = 0,
+.magic = NULL,
+.endian = LV_BIG_ENDIAN,
+
+.versionOffset  = -1,
+.versionSize = 0,
+.versionNumbers = {},
+
+.modeOffset = QCOW2_HDR_CRYPT,
+.modeValue = 2,
+
+.payloadOffset = -1,
+},
 { 0 }
 };
 
-- 
2.26.2

[libvirt PATCH v2 3/4] tests: remove redundant LUKS volume data files

[Daniel P . Berrangé]

The two removed files have exactly the same config as other LUKS volume
data files, simply with different file names. Consolidate down to just
two LUKS volume data files as that's all that we need for the test
coverage.

Signed-off-by: Daniel P. Berrangé 
---
 .../luks-convert-encrypt.argv | 18 
 .../luks-convert-encrypt2fileqcow2.argv   |  6 +++---
 .../luks-convert-encrypt2fileraw.argv |  6 +++---
 tests/storagevolxml2argvtest.c|  8 +++
 tests/storagevolxml2xmlin/vol-encrypt1.xml| 21 ---
 tests/storagevolxml2xmlin/vol-encrypt2.xml| 21 ---
 6 files changed, 19 insertions(+), 61 deletions(-)
 delete mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml
 delete mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml

diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt.argv 
b/tests/storagevolxml2argvdata/luks-convert-encrypt.argv
index b2ad16b7cb..78bce96aaa 100644
--- a/tests/storagevolxml2argvdata/luks-convert-encrypt.argv
+++ b/tests/storagevolxml2argvdata/luks-convert-encrypt.argv
@@ -1,11 +1,11 @@
 qemu-img create -f luks \
---object secret,id=encrypt2.img_encrypt0,file=/path/to/secretFile \
--o key-secret=encrypt2.img_encrypt0 \
-/var/lib/libvirt/images/encrypt2.img 5242880K
+--object secret,id=LuksDemo.img_encrypt0,file=/path/to/secretFile \
+-o key-secret=LuksDemo.img_encrypt0 \
+/var/lib/libvirt/images/LuksDemo.img 5242880K
 qemu-img convert --image-opts -n --target-image-opts \
---object secret,id=encrypt2.img_encrypt0,file=/path/to/secretFile \
---object secret,id=encrypt1.img_encrypt0,file=/path/to/inputSecretFile \
-driver=luks,file.filename=/var/lib/libvirt/images/encrypt1.img,\
-key-secret=encrypt1.img_encrypt0 \
-driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,\
-key-secret=encrypt2.img_encrypt0
+--object secret,id=LuksDemo.img_encrypt0,file=/path/to/secretFile \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/inputSecretFile \
+driver=luks,file.filename=/var/lib/libvirt/images/OtherDemo.img,\
+key-secret=OtherDemo.img_encrypt0 \
+driver=luks,file.filename=/var/lib/libvirt/images/LuksDemo.img,\
+key-secret=LuksDemo.img_encrypt0
diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv 
b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv
index 82cb364b61..fd974f863e 100644
--- a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv
+++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv
@@ -1,7 +1,7 @@
 qemu-img create -f qcow2 \
 -o compat=0.10 /var/lib/libvirt/images/sparse-qcow2.img 1073741824K
 qemu-img convert --image-opts -n --target-image-opts \
---object secret,id=encrypt2.img_encrypt0,file=/path/to/inputSecretFile \
-driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,\
-key-secret=encrypt2.img_encrypt0 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/inputSecretFile \
+driver=luks,file.filename=/var/lib/libvirt/images/OtherDemo.img,\
+key-secret=OtherDemo.img_encrypt0 \
 driver=qcow2,file.filename=/var/lib/libvirt/images/sparse-qcow2.img
diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv 
b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv
index 2661c345a8..82473db57b 100644
--- a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv
+++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv
@@ -1,7 +1,7 @@
 qemu-img create -f raw \
 /var/lib/libvirt/images/sparse.img 1073741824K
 qemu-img convert --image-opts -n --target-image-opts \
---object secret,id=encrypt2.img_encrypt0,file=/path/to/inputSecretFile \
-driver=luks,file.filename=/var/lib/libvirt/images/encrypt2.img,\
-key-secret=encrypt2.img_encrypt0 \
+--object secret,id=OtherDemo.img_encrypt0,file=/path/to/inputSecretFile \
+driver=luks,file.filename=/var/lib/libvirt/images/OtherDemo.img,\
+key-secret=OtherDemo.img_encrypt0 \
 driver=raw,file.filename=/var/lib/libvirt/images/sparse.img
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 1832690e29..618f481039 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -264,16 +264,16 @@ mymain(void)
 "pool-dir", "vol-file-qcow2",
 "luks-convert-qcow2", 0);
 
-DO_TEST("pool-dir", "vol-encrypt2",
-"pool-dir", "vol-encrypt1",
+DO_TEST("pool-dir", "vol-luks",
+"pool-dir", "vol-luks-convert",
 "luks-convert-encrypt", 0);
 
 DO_TEST("pool-dir", "vol-file",
-"pool-dir", "vol-encrypt2",
+"pool-dir", "vol-luks-convert",
 "luks-convert-encrypt2fileraw", 0);
 
 DO_TEST("pool-dir", "vol-file-qcow2",
-"pool-dir", "vol-encrypt2",
+"pool-dir", "vol-luks-convert",
 "luks-convert-encrypt2fileqcow2", 0);
 
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/tests/storagevolxml2xmlin/vol-encrypt1.xml 

[libvirt PATCH v2 0/4] storage: fully support qcow2 with LUKS volumes

[Daniel P . Berrangé]

We added support for LUKS with raw volumes, but never extended it to
cover qcow2 volumes.

In v2:

 - Fixed confusion of src/dst volume types
 - Fixed syntax check
 - Now with unit tests

Daniel P. Berrang=C3=A9 (4):
  scripts: fix logic error in argv wrapping code
  util: detect LUKS encryption scheme in qcow2 files
  tests: remove redundant LUKS volume data files
  storage: add support for qcow2 LUKS encryption

 scripts/test-wrap-argv.py |  2 +-
 src/storage/storage_util.c| 70 ++-
 src/util/virqemu.c| 23 --
 src/util/virqemu.h|  1 +
 src/util/virstoragefile.c | 16 +
 .../luks-convert-encrypt.argv | 18 ++---
 .../luks-convert-encrypt2fileqcow2.argv   |  6 +-
 .../luks-convert-encrypt2fileraw.argv |  6 +-
 .../qcow2-luks-convert-encrypt.argv   | 18 +
 .../qcow2-luks-convert-encrypt2fileqcow2.argv | 14 
 .../qcow2-luks-convert-encrypt2fileraw.argv   | 13 
 tests/storagevolxml2argvdata/qcow2-luks.argv  |  8 +++
 tests/storagevolxml2argvtest.c| 23 --
 ...ncrypt2.xml =3D> vol-qcow2-luks-convert.xml} | 20 --
 .../{vol-encrypt1.xml =3D> vol-qcow2-luks.xml}  | 18 +++--
 tests/storagevolxml2xmlout/vol-qcow2-luks.xml | 31 
 tests/storagevolxml2xmltest.c |  1 +
 17 files changed, 237 insertions(+), 51 deletions(-)
 create mode 100644 tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.a=
rgv
 create mode 100644 tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2f=
ileqcow2.argv
 create mode 100644 tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2f=
ileraw.argv
 create mode 100644 tests/storagevolxml2argvdata/qcow2-luks.argv
 rename tests/storagevolxml2xmlin/{vol-encrypt2.xml =3D> vol-qcow2-luks-conve=
rt.xml} (50%)
 rename tests/storagevolxml2xmlin/{vol-encrypt1.xml =3D> vol-qcow2-luks.xml} =
(52%)
 create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-luks.xml

--=20
2.26.2

[libvirt PATCH v2 1/4] scripts: fix logic error in argv wrapping code

[Daniel P . Berrangé]

The first piece of the command we process must be added to the list
straight away regardless of whether it starts with a '-' or not.

Signed-off-by: Daniel P. Berrangé 
---
 scripts/test-wrap-argv.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/test-wrap-argv.py b/scripts/test-wrap-argv.py
index 4193e6b68d..6b0d3511f3 100755
--- a/scripts/test-wrap-argv.py
+++ b/scripts/test-wrap-argv.py
@@ -59,7 +59,7 @@ def rewrap_line(line):
 # If there's a leading '-' then this is a new
 # parameter, otherwise its a value for the prev
 # parameter.
-if bit.startswith("-"):
+if bit.startswith("-") or len(args) == 0:
 args.append(bit)
 else:
 args[-1] = args[-1] + " " + bit
-- 
2.26.2

Re: [PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes

[Andrea Bolognani]

On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:
> In [1], changes were made to remove the existing auto-alignment
> for pSeries NVDIMM devices. That design promotes strange situations
> where the NVDIMM size reported in the domain XML is different
> from what QEMU is actually using. We removed the auto-alignment
> and relied on standard size validation.
> 
> However, this goes against Libvirt design philosophy of not
> tampering with existing guest behavior, as pointed out by Daniel
> in [2]. Since we can't know for sure whether there are guests that
> are relying on the auto-alignment feature to work, the changes
> made in [1] are a direct violation of this rule.
> 
> This patch reverts [1] entirely, re-enabling auto-alignment for
> pSeries NVDIMM as it was before. Changes will be made to ease
> the limitations of this design without hurting existing
> guests.
> 
> This reverts the following commits:
> 
> - commit 0ee56369c8b4f2f898b6aa1ff1f51ab033be1c02,
> "qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type".
> 
> - commit 07de813924caf37e535855541c0c1183d9d382e2,
> "qemu_domain.c: do not auto-align ppc64 NVDIMMs"
> 
> - commit 0ccceaa57c50e5ee528f7073fa8723afd62b88b7,
> "qemu_validate.c: add pSeries NVDIMM size alignment validation"
> 
> - commit 4fa2202d884414ad34d9952e72fb39b1d93c7e14,
> "qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public"
> 
> - commit 2d93cbdea9d1b8dbf36bc0ffee6cb73d83d208c7,
> "Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic""
> 
> [1] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html
> [2] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html

Reverting multiple commits with a single commit is sort of unusual,
but in this case it seem to make sense and I don't really have a
problem with it, so unless someone shouts against this approach I
think we can merge it as-is.

A few tweaks to the commit message, though:

  * keep the order of commits consistent to the one they were merged
in, which more specifically means putting 2d93cbdea9d1 at the
very top of the list;

  * lose the commas after the various commit hashes - they break
convenient double-click selection in most terminals;

  * indent the commit subject by two spaces for better readability.

The commit is also missing your S-o-b, and as you know that's a
blocker for merging.

With the above addressed,

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt] [PATCH v2 0/4] qemu/virsh/docs: various minor fixes

[Michal Privoznik]

On 9/17/20 5:20 AM, Lin Ma wrote:

On 2020-09-16 10:40, Michal Privoznik wrote:

On 9/16/20 9:17 AM, moreca...@gmail.com wrote:

From: Lin Ma 

v1 -> v2:

* Remove the merged patches.
* Follow Ján and Erik's suggestions.

Lin Ma (4):
   virsh: net-port-create: log errors for non-existent xml file
   virsh: event: options --list, --all and --event are mutually 
exclusive

   docs: virsh: Document the IO mode 'io_uring'
   network: Check for active network during networkGetDHCPLeases

  docs/manpages/virsh.rst | 3 ++-
  src/network/bridge_driver.c | 7 +++
  tools/virsh-domain.c    | 4 
  tools/virsh-network.c   | 1 +
  4 files changed, 14 insertions(+), 1 deletion(-)



Patches looks good to me, but see my comments. I can do the changes
before pushing, if you agree with suggested changes.


No problem, Thank you very much.



Since Jano didn't like 4/4, I'm pushing 1-3/4 only.

Reviewed-by: Michal Privoznik 

Michal

Re: [PATCH 1/2] docs: formatdomain: Add examples for nbd source

[Han Han]

Peter, could you please review and merge this patch first?
It is actually independent from the 2/2 rejected patch.

On Wed, Sep 16, 2020 at 1:49 PM Han Han  wrote:

> Signed-off-by: Han Han 
> ---
>  docs/formatdomain.rst | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 18e237c157..49713a12d4 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2409,6 +2409,27 @@ paravirtualized driver is specified via the
> ``disk`` element.
> 
> 
>   
> + 
> +   
> +   
> + 
> +   
> +   
> + 
> + 
> +   
> +   
> + 
> +   
> +   
> + 
> + 
> +   
> +   
> + 
> +   
> +   
> + 
> 
> ...
>
> --
> 2.28.0
>
>

[PATCH v2 4/6] qemuBuildMemoryBackendProps: Fix const correctness

[Michal Privoznik]

The objects at @def and @mem pointers are only read and not
written. Make the arguments const to make that explicit.

Signed-off-by: Michal Privoznik 
Reviewed-by: Ján Tomko 
---
 src/qemu/qemu_command.c | 4 ++--
 src/qemu/qemu_command.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1d87f43d61..421589ea85 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2976,8 +2976,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
 const char *alias,
 virQEMUDriverConfigPtr cfg,
 qemuDomainObjPrivatePtr priv,
-virDomainDefPtr def,
-virDomainMemoryDefPtr mem,
+const virDomainDef *def,
+const virDomainMemoryDef *mem,
 bool force)
 {
 const char *backendType = "memory-backend-file";
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 89d99b111f..8a30f2852c 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -150,8 +150,8 @@ int qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
 const char *alias,
 virQEMUDriverConfigPtr cfg,
 qemuDomainObjPrivatePtr priv,
-virDomainDefPtr def,
-virDomainMemoryDefPtr mem,
+const virDomainDef *def,
+const virDomainMemoryDef *mem,
 bool force);
 
 char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
-- 
2.26.2

[PATCH v2 6/6] qemu: Use memory-backend-* for regular guest memory

[Michal Privoznik]

So far, Libvirt configures memory-backend-* for memory hotplug,
possibly NUMA nodes and in a few other cases. This patch
switches to constructing the memory-backend-* command line for
all cases. To keep ability to migrate guests a little hack is
used: the ID of the object is set to the one that QEMU uses
internally anyways. These IDs are stable (first started to appear
somewhere around v0.13.0-rc0~96) and can't change.

In fact, this patch does exactly what QEMU does internally. The
reason for moving the logic into Libvirt is that QEMU wants to
deprecate the old style of specifying memory.

So far, only x84_64 test cases are changed, because tests for
other architectures use older capabilities, which still lack the
QEMU_CAPS_MACHINE_MEMORY_BACKEND capability and they don't report
the RAM ID.

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

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c   | 67 ---
 .../blkdeviotune-group-num.x86_64-latest.args |  3 +-
 ...blkdeviotune-max-length.x86_64-latest.args |  3 +-
 .../blkdeviotune-max.x86_64-latest.args   |  3 +-
 .../channel-unix-guestfwd.x86_64-latest.args  |  3 +-
 .../console-virtio-unix.x86_64-latest.args|  3 +-
 .../controller-virtio-scsi.x86_64-latest.args |  3 +-
 ...-Icelake-Server-pconfig.x86_64-latest.args |  3 +-
 .../cpu-translation.x86_64-latest.args|  3 +-
 .../cputune-cpuset-big-id.x86_64-latest.args  |  3 +-
 .../disk-aio-io_uring.x86_64-latest.args  |  3 +-
 .../disk-aio.x86_64-latest.args   |  3 +-
 ...-backing-chains-noindex.x86_64-latest.args |  3 +-
 .../disk-cache.x86_64-latest.args |  4 +-
 .../disk-cdrom-bus-other.x86_64-latest.args   |  3 +-
 ...m-empty-network-invalid.x86_64-latest.args |  3 +-
 .../disk-cdrom-network.x86_64-latest.args |  3 +-
 .../disk-cdrom-tray.x86_64-latest.args|  3 +-
 .../disk-cdrom.x86_64-latest.args |  3 +-
 .../disk-copy_on_read.x86_64-latest.args  |  3 +-
 .../disk-detect-zeroes.x86_64-latest.args |  3 +-
 .../disk-discard.x86_64-latest.args   |  3 +-
 .../disk-error-policy.x86_64-latest.args  |  3 +-
 .../disk-floppy-q35-2_11.x86_64-latest.args   |  4 +-
 .../disk-floppy-q35-2_9.x86_64-latest.args|  4 +-
 .../disk-floppy.x86_64-latest.args|  3 +-
 .../disk-network-gluster.x86_64-latest.args   |  3 +-
 .../disk-network-http.x86_64-latest.args  |  3 +-
 .../disk-network-iscsi.x86_64-latest.args |  3 +-
 .../disk-network-nbd.x86_64-latest.args   |  3 +-
 .../disk-network-rbd.x86_64-latest.args   |  3 +-
 .../disk-network-sheepdog.x86_64-latest.args  |  3 +-
 ...isk-network-source-auth.x86_64-latest.args |  3 +-
 ...isk-network-tlsx509-nbd.x86_64-latest.args |  3 +-
 .../disk-nvme.x86_64-latest.args  |  3 +-
 .../disk-readonly-disk.x86_64-latest.args |  3 +-
 .../disk-scsi-device-auto.x86_64-latest.args  |  3 +-
 .../disk-scsi.x86_64-latest.args  |  3 +-
 .../disk-shared.x86_64-latest.args|  3 +-
 .../disk-slices.x86_64-latest.args|  3 +-
 ...irtio-scsi-reservations.x86_64-latest.args |  3 +-
 .../eoi-disabled.x86_64-latest.args   |  3 +-
 .../eoi-enabled.x86_64-latest.args|  3 +-
 .../floppy-drive-fat.x86_64-latest.args   |  3 +-
 .../qemuxml2argvdata/fs9p.x86_64-latest.args  |  3 +-
 .../genid-auto.x86_64-latest.args |  3 +-
 .../qemuxml2argvdata/genid.x86_64-latest.args |  3 +-
 ...egl-headless-rendernode.x86_64-latest.args |  3 +-
 .../graphics-egl-headless.x86_64-latest.args  |  3 +-
 ...pice-gl-auto-rendernode.x86_64-latest.args |  3 +-
 ...graphics-vnc-tls-secret.x86_64-latest.args |  3 +-
 .../graphics-vnc-tls.x86_64-latest.args   |  3 +-
 ...tdev-mdev-display-ramfb.x86_64-latest.args |  3 +-
 ...play-spice-egl-headless.x86_64-latest.args |  3 +-
 ...ev-display-spice-opengl.x86_64-latest.args |  3 +-
 ...isplay-vnc-egl-headless.x86_64-latest.args |  3 +-
 ...ostdev-mdev-display-vnc.x86_64-latest.args |  3 +-
 .../hostdev-scsi-lsi.x86_64-latest.args   |  3 +-
 ...ostdev-scsi-virtio-scsi.x86_64-latest.args |  3 +-
 .../hugepages-nvdimm.x86_64-latest.args   |  6 +-
 .../hyperv-off.x86_64-latest.args |  3 +-
 .../hyperv-panic.x86_64-latest.args   |  3 +-
 .../hyperv-stimer-direct.x86_64-latest.args   |  3 +-
 .../hyperv.x86_64-latest.args |  3 +-
 .../intel-iommu-aw-bits.x86_64-latest.args|  4 +-
 ...ntel-iommu-caching-mode.x86_64-latest.args |  4 +-
 ...ntel-iommu-device-iotlb.x86_64-latest.args |  4 +-
 .../intel-iommu-eim.x86_64-latest.args|  4 +-
 .../intel-iommu.x86_64-latest.args|  3 +-
 ...threads-virtio-scsi-pci.x86_64-latest.args |  3 +-
 .../kvmclock+eoi-disabled.x86_64-latest.args  |  3 +-
 ...luks-disks-source-qcow2.x86_64-latest.args |  4 +-
 ...memory-default-hugepage.x86_64-latest.args |  8 ++-
 .../memfd-memory-numa.x86_64-latest.args  |  8 ++-
 

[PATCH v2 3/6] qemuBuildMemoryBackendProps: Prealloc mem for memfd backend

[Michal Privoznik]

If a domain was using hugepages through memory-backend-file or
via -mem-path, we would turn prealloc on. But we are not doing
that for memory-backend-memfd. Fix this, because we need QEMU to
fully allocate hugepages.

Signed-off-by: Michal Privoznik 
Reviewed-by: Ján Tomko 
---
 src/qemu/qemu_command.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b690d54baa..1d87f43d61 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3097,10 +3097,13 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
 if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) {
 backendType = "memory-backend-memfd";
 
-if (useHugepage &&
-(virJSONValueObjectAdd(props, "b:hugetlb", useHugepage, NULL) < 0 
||
- virJSONValueObjectAdd(props, "U:hugetlbsize", pagesize << 10, 
NULL) < 0)) {
-return -1;
+if (useHugepage) {
+if (virJSONValueObjectAdd(props, "b:hugetlb", useHugepage, NULL) < 
0 ||
+virJSONValueObjectAdd(props, "U:hugetlbsize", pagesize << 10, 
NULL) < 0) {
+return -1;
+}
+
+prealloc = true;
 }
 
 if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
-- 
2.26.2

[PATCH v2 0/6] qemu: Use memory-backend-* for regular guest memory

[Michal Privoznik]

v2 of:

https://www.redhat.com/archives/libvir-list/2020-September/msg00023.html

diff to v1:
- I've dropped the capability (patch 5/7 in the original set) and am
  relying on qemu providing default-ram-id.


I've tested migration from a daemon with these patches to a daemon
without and also vice versa and it worked.

Michal Prívozník (6):
  qemuBuildMemoryBackendProps: Move @prealloc setting to backend
agnostic part
  qemuBuildMemoryBackendProps: Respect
//memoryBacking/allocation/@mode=immediate
  qemuBuildMemoryBackendProps: Prealloc mem for memfd backend
  qemuBuildMemoryBackendProps: Fix const correctness
  qemu: Track default-ram-id machine attribute
  qemu: Use memory-backend-* for regular guest memory

 src/qemu/qemu_capabilities.c  |  35 +++-
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_capspriv.h  |   3 +-
 src/qemu/qemu_command.c   |  96 +++--
 src/qemu/qemu_command.h   |   4 +-
 src/qemu/qemu_monitor.c   |   1 +
 src/qemu/qemu_monitor.h   |   1 +
 src/qemu/qemu_monitor_json.c  |  11 +
 .../caps_5.2.0.x86_64.xml | 196 +-
 .../blkdeviotune-group-num.x86_64-latest.args |   3 +-
 ...blkdeviotune-max-length.x86_64-latest.args |   3 +-
 .../blkdeviotune-max.x86_64-latest.args   |   3 +-
 .../channel-unix-guestfwd.x86_64-latest.args  |   3 +-
 .../console-virtio-unix.x86_64-latest.args|   3 +-
 .../controller-virtio-scsi.x86_64-latest.args |   3 +-
 ...-Icelake-Server-pconfig.x86_64-latest.args |   3 +-
 .../cpu-translation.x86_64-latest.args|   3 +-
 .../cputune-cpuset-big-id.x86_64-latest.args  |   3 +-
 .../disk-aio-io_uring.x86_64-latest.args  |   3 +-
 .../disk-aio.x86_64-latest.args   |   3 +-
 ...-backing-chains-noindex.x86_64-latest.args |   3 +-
 .../disk-cache.x86_64-latest.args |   4 +-
 .../disk-cdrom-bus-other.x86_64-latest.args   |   3 +-
 ...m-empty-network-invalid.x86_64-latest.args |   3 +-
 .../disk-cdrom-network.x86_64-latest.args |   3 +-
 .../disk-cdrom-tray.x86_64-latest.args|   3 +-
 .../disk-cdrom.x86_64-latest.args |   3 +-
 .../disk-copy_on_read.x86_64-latest.args  |   3 +-
 .../disk-detect-zeroes.x86_64-latest.args |   3 +-
 .../disk-discard.x86_64-latest.args   |   3 +-
 .../disk-error-policy.x86_64-latest.args  |   3 +-
 .../disk-floppy-q35-2_11.x86_64-latest.args   |   4 +-
 .../disk-floppy-q35-2_9.x86_64-latest.args|   4 +-
 .../disk-floppy.x86_64-latest.args|   3 +-
 .../disk-network-gluster.x86_64-latest.args   |   3 +-
 .../disk-network-http.x86_64-latest.args  |   3 +-
 .../disk-network-iscsi.x86_64-latest.args |   3 +-
 .../disk-network-nbd.x86_64-latest.args   |   3 +-
 .../disk-network-rbd.x86_64-latest.args   |   3 +-
 .../disk-network-sheepdog.x86_64-latest.args  |   3 +-
 ...isk-network-source-auth.x86_64-latest.args |   3 +-
 ...isk-network-tlsx509-nbd.x86_64-latest.args |   3 +-
 .../disk-nvme.x86_64-latest.args  |   3 +-
 .../disk-readonly-disk.x86_64-latest.args |   3 +-
 .../disk-scsi-device-auto.x86_64-latest.args  |   3 +-
 .../disk-scsi.x86_64-latest.args  |   3 +-
 .../disk-shared.x86_64-latest.args|   3 +-
 .../disk-slices.x86_64-latest.args|   3 +-
 ...irtio-scsi-reservations.x86_64-latest.args |   3 +-
 .../eoi-disabled.x86_64-latest.args   |   3 +-
 .../eoi-enabled.x86_64-latest.args|   3 +-
 .../floppy-drive-fat.x86_64-latest.args   |   3 +-
 .../qemuxml2argvdata/fs9p.x86_64-latest.args  |   3 +-
 .../genid-auto.x86_64-latest.args |   3 +-
 .../qemuxml2argvdata/genid.x86_64-latest.args |   3 +-
 ...egl-headless-rendernode.x86_64-latest.args |   3 +-
 .../graphics-egl-headless.x86_64-latest.args  |   3 +-
 ...pice-gl-auto-rendernode.x86_64-latest.args |   3 +-
 ...graphics-vnc-tls-secret.x86_64-latest.args |   3 +-
 .../graphics-vnc-tls.x86_64-latest.args   |   3 +-
 ...tdev-mdev-display-ramfb.x86_64-latest.args |   3 +-
 ...play-spice-egl-headless.x86_64-latest.args |   3 +-
 ...ev-display-spice-opengl.x86_64-latest.args |   3 +-
 ...isplay-vnc-egl-headless.x86_64-latest.args |   3 +-
 ...ostdev-mdev-display-vnc.x86_64-latest.args |   3 +-
 .../hostdev-scsi-lsi.x86_64-latest.args   |   3 +-
 ...ostdev-scsi-virtio-scsi.x86_64-latest.args |   3 +-
 .../qemuxml2argvdata/hugepages-memaccess.args |  30 +--
 .../hugepages-memaccess2.args |  12 +-
 .../hugepages-numa-nodeset-part.args  |   5 +-
 .../hugepages-numa-nodeset.args   |  20 +-
 .../hugepages-nvdimm.x86_64-latest.args   |  15 +-
 tests/qemuxml2argvdata/hugepages-shared.args  |  24 +--
 .../hyperv-off.x86_64-latest.args |   3 +-
 .../hyperv-panic.x86_64-latest.args   |   3 +-
 .../hyperv-stimer-direct.x86_64-latest.args   

[PATCH v2 1/6] qemuBuildMemoryBackendProps: Move @prealloc setting to backend agnostic part

[Michal Privoznik]

All three memory backends (-file, -ram and -memfd) have .prealloc
attribute. Since we are setting it only for -file, the
corresponding code lives only under if() that handles that
specific backend. But in near future we will want to set the
attribute for other backends too. Therefore, move the
corresponding code outside of the if().

This causes some .argv files to be changed, but the only change
happening there is move of the attribute (best viewed with:
'git show --color-words=.').

Signed-off-by: Michal Privoznik 
Reviewed-by: Ján Tomko 
---
 src/qemu/qemu_command.c   | 11 +++
 .../qemuxml2argvdata/hugepages-memaccess.args | 30 +--
 .../hugepages-memaccess2.args | 12 
 .../hugepages-numa-nodeset-part.args  |  5 ++--
 .../hugepages-numa-nodeset.args   | 20 -
 .../hugepages-nvdimm.x86_64-latest.args   |  9 +++---
 tests/qemuxml2argvdata/hugepages-shared.args  | 24 +++
 .../memory-hotplug-dimm-addr.args |  6 ++--
 .../qemuxml2argvdata/memory-hotplug-dimm.args |  6 ++--
 ...y-hotplug-nvdimm-access.x86_64-latest.args |  4 +--
 ...ry-hotplug-nvdimm-align.x86_64-latest.args |  4 +--
 ...ry-hotplug-nvdimm-label.x86_64-latest.args |  4 +--
 ...ory-hotplug-nvdimm-pmem.x86_64-latest.args |  4 +--
 ...ory-hotplug-nvdimm-ppc64.ppc64-latest.args |  2 +-
 ...hotplug-nvdimm-readonly.x86_64-latest.args |  4 +--
 .../memory-hotplug-nvdimm.x86_64-latest.args  |  2 +-
 .../qemuxml2argvdata/pages-dimm-discard.args  |  4 +--
 tests/qemuxml2argvdata/user-aliases.args  | 16 +-
 ...vhost-user-fs-hugepages.x86_64-latest.args |  5 ++--
 19 files changed, 90 insertions(+), 82 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0ba348e911..2ff6da9236 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3108,13 +3108,11 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
 
 if (mem->nvdimmPath) {
 memPath = g_strdup(mem->nvdimmPath);
-if (!priv->memPrealloc)
-prealloc = true;
+prealloc = true;
 } else if (useHugepage) {
 if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, 
) < 0)
 return -1;
-if (!priv->memPrealloc)
-prealloc = true;
+prealloc = true;
 } else {
 /* We can have both pagesize and mem source. If that's the case,
  * prefer hugepages as those are more specific. */
@@ -3123,7 +3121,6 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
 }
 
 if (virJSONValueObjectAdd(props,
-  "B:prealloc", prealloc,
   "s:mem-path", memPath,
   NULL) < 0)
 return -1;
@@ -3148,6 +3145,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
 backendType = "memory-backend-ram";
 }
 
+if (!priv->memPrealloc &&
+virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
+return -1;
+
 if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0)
 return -1;
 
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess.args 
b/tests/qemuxml2argvdata/hugepages-memaccess.args
index 7cfbce7c7c..3358a8c965 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess.args
+++ b/tests/qemuxml2argvdata/hugepages-memaccess.args
@@ -14,25 +14,25 @@ QEMU_AUDIO_DRV=none \
 -m size=4194304k,slots=16,maxmem=8388608k \
 -realtime mlock=off \
 -smp 4,sockets=4,cores=1,threads=1 \
--object memory-backend-file,id=ram-node0,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu/-1-QEMUGuest1,share=no,size=1073741824,\
-host-nodes=0-3,policy=bind \
+-object memory-backend-file,id=ram-node0,\
+mem-path=/dev/hugepages1G/libvirt/qemu/-1-QEMUGuest1,share=no,prealloc=yes,\
+size=1073741824,host-nodes=0-3,policy=bind \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
--object memory-backend-file,id=ram-node1,prealloc=yes,\
-mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,size=1073741824,\
-host-nodes=0-3,policy=bind \
+-object memory-backend-file,id=ram-node1,\
+mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,prealloc=yes,\
+size=1073741824,host-nodes=0-3,policy=bind \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
--object memory-backend-file,id=ram-node2,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu/-1-QEMUGuest1,share=no,size=1073741824,\
-host-nodes=0-3,policy=bind \
+-object memory-backend-file,id=ram-node2,\
+mem-path=/dev/hugepages1G/libvirt/qemu/-1-QEMUGuest1,share=no,prealloc=yes,\
+size=1073741824,host-nodes=0-3,policy=bind \
 -numa node,nodeid=2,cpus=2,memdev=ram-node2 \
--object memory-backend-file,id=ram-node3,prealloc=yes,\
-mem-path=/dev/hugepages1G/libvirt/qemu/-1-QEMUGuest1,share=no,size=1073741824,\
-host-nodes=3,policy=bind \
+-object 

[PATCH v2 5/6] qemu: Track default-ram-id machine attribute

[Michal Privoznik]

The machine structure has another (optional) attribute:
default-ram-id, which specifies the alias of the default RAM
object. While the alias is private, it can never change in order
to not break migration. QEMU uses the alias when allocating
regular, not NUMA memory. In order to switch to new command line
and maintain migration, save this ID.

Signed-off-by: Michal Privoznik 
Reviewed-by: Ján Tomko 
---
 src/qemu/qemu_capabilities.c  |  35 +++-
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_capspriv.h  |   3 +-
 src/qemu/qemu_monitor.c   |   1 +
 src/qemu/qemu_monitor.h   |   1 +
 src/qemu/qemu_monitor_json.c  |  11 +
 .../caps_5.2.0.x86_64.xml | 196 +-
 tests/testutilsqemu.c |  24 ++-
 8 files changed, 170 insertions(+), 104 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2cc0c61588..41c0d4866f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -610,6 +610,7 @@ struct _virQEMUCapsMachineType {
 bool qemuDefault;
 char *defaultCPU;
 bool numaMemSupported;
+char *defaultRAMid;
 };
 
 typedef struct _virQEMUCapsHostCPUData virQEMUCapsHostCPUData;
@@ -1888,6 +1889,7 @@ virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccelPtr dst,
 dst->machineTypes[i].hotplugCpus = src->machineTypes[i].hotplugCpus;
 dst->machineTypes[i].qemuDefault = src->machineTypes[i].qemuDefault;
 dst->machineTypes[i].numaMemSupported = 
src->machineTypes[i].numaMemSupported;
+dst->machineTypes[i].defaultRAMid = 
g_strdup(src->machineTypes[i].defaultRAMid);
 }
 }
 
@@ -1965,6 +1967,7 @@ virQEMUCapsAccelClear(virQEMUCapsAccelPtr caps)
 VIR_FREE(caps->machineTypes[i].name);
 VIR_FREE(caps->machineTypes[i].alias);
 VIR_FREE(caps->machineTypes[i].defaultCPU);
+VIR_FREE(caps->machineTypes[i].defaultRAMid);
 }
 VIR_FREE(caps->machineTypes);
 
@@ -2551,6 +2554,25 @@ virQEMUCapsGetMachineNumaMemSupported(virQEMUCapsPtr 
qemuCaps,
 }
 
 
+const char *
+virQEMUCapsGetMachineDefaultRAMid(virQEMUCapsPtr qemuCaps,
+  virDomainVirtType virtType,
+  const char *name)
+{
+virQEMUCapsAccelPtr accel;
+size_t i;
+
+accel = virQEMUCapsGetAccel(qemuCaps, virtType);
+
+for (i = 0; i < accel->nmachineTypes; i++) {
+if (STREQ(accel->machineTypes[i].name, name))
+return accel->machineTypes[i].defaultRAMid;
+}
+
+return NULL;
+}
+
+
 /**
  * virQEMUCapsSetGICCapabilities:
  * @qemuCaps: QEMU capabilities
@@ -2787,7 +2809,8 @@ virQEMUCapsAddMachine(virQEMUCapsPtr qemuCaps,
   int maxCpus,
   bool hotplugCpus,
   bool isDefault,
-  bool numaMemSupported)
+  bool numaMemSupported,
+  const char *defaultRAMid)
 {
 virQEMUCapsAccelPtr accel = virQEMUCapsGetAccel(qemuCaps, virtType);
 virQEMUCapsMachineTypePtr mach;
@@ -2808,6 +2831,8 @@ virQEMUCapsAddMachine(virQEMUCapsPtr qemuCaps,
 mach->qemuDefault = isDefault;
 
 mach->numaMemSupported = numaMemSupported;
+
+mach->defaultRAMid = g_strdup(defaultRAMid);
 }
 
 /**
@@ -2854,7 +2879,8 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
   machines[i]->maxCpus,
   machines[i]->hotplugCpus,
   machines[i]->isDefault,
-  machines[i]->numaMemSupported);
+  machines[i]->numaMemSupported,
+  machines[i]->defaultRAMid);
 
 if (preferredMachine &&
 (STREQ_NULLABLE(machines[i]->alias, preferredMachine) ||
@@ -4081,6 +4107,7 @@ virQEMUCapsLoadMachines(virQEMUCapsAccelPtr caps,
 VIR_FREE(str);
 
 caps->machineTypes[i].defaultCPU = virXMLPropString(nodes[i], 
"defaultCPU");
+caps->machineTypes[i].defaultRAMid = virXMLPropString(nodes[i], 
"defaultRAMid");
 }
 
 return 0;
@@ -4569,6 +4596,8 @@ virQEMUCapsFormatMachines(virQEMUCapsAccelPtr caps,
   caps->machineTypes[i].defaultCPU);
 if (caps->machineTypes[i].numaMemSupported)
 virBufferAddLit(buf, " numaMemSupported='yes'");
+virBufferEscapeString(buf, " defaultRAMid='%s'",
+  caps->machineTypes[i].defaultRAMid);
 virBufferAddLit(buf, "/>\n");
 }
 }
@@ -6378,7 +6407,7 @@ virQEMUCapsStripMachineAliasesForVirtType(virQEMUCapsPtr 
qemuCaps,
 if (name) {
 virQEMUCapsAddMachine(qemuCaps, virtType, name, NULL, 
mach->defaultCPU,
   mach->maxCpus, mach->hotplugCpus, 
mach->qemuDefault,
-  mach->numaMemSupported);

[PATCH v2 2/6] qemuBuildMemoryBackendProps: Respect //memoryBacking/allocation/@mode=immediate

[Michal Privoznik]

If user specifies immediate memory allocation in the domain XML,
they want QEMU to fully allocate its memory. And if the memory
was allocated using plain '-m' then we would honour it. But, if a
memory backend is used, then we don't set the prealloc attribute
of the backend.

Signed-off-by: Michal Privoznik 
Reviewed-by: Ján Tomko 
---
 src/qemu/qemu_command.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2ff6da9236..b690d54baa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3027,6 +3027,9 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
 if (discard == VIR_TRISTATE_BOOL_ABSENT)
 discard = def->mem.discard;
 
+if (def->mem.allocation == VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE)
+prealloc = true;
+
 if (virDomainNumatuneGetMode(def->numa, mem->targetNode, ) < 0 &&
 virDomainNumatuneGetMode(def->numa, -1, ) < 0)
 mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
-- 
2.26.2

Re: [libvirt PATCH 2/2] storage: add support for qcow2 LUKS encryption

[Han Han]

On Thu, Sep 17, 2020 at 1:06 AM Daniel P. Berrangé 
wrote:

> The storage driver was wired up to support creating raw volumes in LUKS
> format, but was never adapted to support LUKS-in-qcow2. This is trivial
> as it merely requires the encryption properties to be prefixed with
> the "encrypt." prefix, and "encrypt.format=luks" when creating the
> volume.
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/storage/storage_util.c | 70 +-
>  src/util/virqemu.c | 23 +
>  src/util/virqemu.h |  1 +
>  3 files changed, 72 insertions(+), 22 deletions(-)
>
> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> index cf82ea0a87..e5e4fe428f 100644
> --- a/src/storage/storage_util.c
> +++ b/src/storage/storage_util.c
> @@ -707,7 +707,7 @@
> storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo,
>
>  virStorageFileFormatTypeToString(info->backingFormat));
>
>  if (encinfo)
> -virQEMUBuildQemuImgKeySecretOpts(, encinfo,
> info->secretAlias);
> +virQEMUBuildQemuImgKeySecretOpts(, info->format, encinfo,
> info->secretAlias);
>
>  if (info->preallocate) {
>  if (info->size_arg > info->allocation)
> @@ -761,7 +761,8 @@ storageBackendCreateQemuImgCheckEncryption(int format,
>  {
>  virStorageEncryptionPtr enc = vol->target.encryption;
>
> -if (format == VIR_STORAGE_FILE_RAW) {
> +if (format == VIR_STORAGE_FILE_RAW ||
> +format == VIR_STORAGE_FILE_QCOW2) {
>  if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unsupported volume encryption format %d"),
> @@ -927,21 +928,34 @@
> storageBackendCreateQemuImgSecretObject(virCommandPtr cmd,
>  }
>
>
> -/* Add a --image-opts to the qemu-img resize command line:
> +/* Add a --image-opts to the qemu-img resize command line for use
> + * with encryption:
>   *--image-opts
> driver=luks,file.filename=$volpath,key-secret=$secretAlias
> + * or
> + *--image-opts
> driver=qcow2,file.filename=$volpath,encrypt.key-secret=$secretAlias
>   *
> - *NB: format=raw is assumed
>   */
>  static int
>  storageBackendResizeQemuImgImageOpts(virCommandPtr cmd,
> + int format,
>   const char *path,
>   const char *secretAlias)
>  {
>  g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>  g_autofree char *commandStr = NULL;
> +const char *encprefix;
> +const char *driver;
>
> -virBufferAsprintf(, "driver=luks,key-secret=%s,file.filename=",
> -  secretAlias);
> +if (format == VIR_STORAGE_FILE_QCOW2) {
> +driver = "qcow2";
> +encprefix = "encrypt.";
> +} else {
> +driver = "luks";
> +encprefix = "";
> +}
> +
> +virBufferAsprintf(, "driver=%s,%skey-secret=%s,file.filename=",
> +  driver, encprefix, secretAlias);
>  virQEMUBuildBufferEscapeComma(, path);
>
>  commandStr = virBufferContentAndReset();
> @@ -1006,6 +1020,16 @@
> virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
>  return -1;
>  }
>  }
>
storagevolxml2argvtest gets segment fault at this function:
➜  ~ abs_top_srcdir='/root/libvirt-6.7.0' LC_ALL='C'
abs_top_builddir='/root/libvirt-6.7.0/build'
abs_srcdir='/root/libvirt-6.7.0/tests'
abs_builddir='/root/libvirt-6.7.0/build/tests' VIR_TEST_EXPENSIVE='0'
LIBVIRT_AUTOSTART='0'
/root/libvirt-6.7.0/build/tests/storagevolxml2argvtest




TEST: storagevolxml2argvtest
  ...![1]916320 segmentation fault (core dumped)
 abs_top_srcdir='/root/libvirt-6.7.0' LC_ALL='C' abs_top_builddir=
abs_srcdir=


Backtrace:
(gdb) bt
#0  0x558b997ea149 in virStorageBackendCreateQemuImgSetInfo
(info=, convertStep=VIR_STORAGE_VOL_ENCRYPT_CREATE,
inputvol=0x558b9b32e810, vol=0x558b9b32f840, pool=0x558b9b323b30)

at ../src/storage/storage_util.c:1025
#1  0x558b997ea149 in virStorageBackendCreateQemuImgCmdFromVol
(pool=0x558b9b323b30, vol=0x558b9b32f840, inputvol=0x558b9b32e810,
flags=0, create_tool=0x558b997f6a00  "qemu-img",
secretPath=0x558b997f6537 "/path/to/secretFile",
inputSecretPath=0x558b997f654b "/path/to/inputSecretFile",
convertStep=VIR_STORAGE_VOL_ENCRYPT_CREATE) at
../src/storage/storage_util.c:1103
#2  0x558b997e4b57 in testCompareXMLToArgvFiles
(parse_flags=, flags=0, cmdline=0x558b9b325030
"/root/libvirt-6.7.0/tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv",
inputvolxml=0x558b9b32f7f0
"/root/libvirt-6.7.0/tests/storagevolxml2xmlin/vol-encrypt2.xml",
inputpoolxml=0x558b9b331730
"/root/libvirt-6.7.0/tests/storagepoolxml2xmlin/pool-dir.xml",
volxml=0x558b9b3309f0
"/root/libvirt-6.7.0/tests/storagevolxml2xmlin/vol-file.xml",
poolxml=0x558b9b32a530
"/root/libvirt-6.7.0/tests/storagepoolxml2xmlin/pool-dir.xml",
shouldFail=false) at 

Re: [libvirt PATCH] remote: slightly improve debugging of socket selection

[Andrea Bolognani]

On Wed, 2020-09-16 at 18:05 +0100, Daniel P. Berrangé wrote:
> The current debug message reports the "mode" after selection has
> completed, however, the "mode" value can be changed by the selection
> logic. It is thus beneficial to report most values upfront, and only
> report newly changed values at the end.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/remote/remote_sockets.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH 03/14] util: introduce helpers for GLib DBus implementation

[Pavel Hrdina]

With libdbus our wrappers had a special syntax to create the DBus
messages by defining the DBus message signature followed by list
of arguments providing data based on the signature.

There will be no similar helper with GLib implementation as they
provide same functionality via GVariant APIs. The syntax is slightly
different mostly for how arrays, variadic types and dictionaries are
created/parsed.

Additional difference is that with GLib DBus everything is wrapped in
extra tuple (struct). For more details refer to the documentation [1].

[1] 

Signed-off-by: Pavel Hrdina 
---
 meson.build  |   6 +-
 po/POTFILES.in   |   1 +
 src/libvirt_private.syms |  14 ++
 src/util/meson.build |   1 +
 src/util/virgdbus.c  | 425 +++
 src/util/virgdbus.h  |  79 
 6 files changed, 525 insertions(+), 1 deletion(-)
 create mode 100644 src/util/virgdbus.c
 create mode 100644 src/util/virgdbus.h

diff --git a/meson.build b/meson.build
index 3d15b4ee34..b5b1223227 100644
--- a/meson.build
+++ b/meson.build
@@ -1066,7 +1066,11 @@ endif
 glib_version = '2.48.0'
 glib_dep = dependency('glib-2.0', version: '>=' + glib_version)
 gobject_dep = dependency('gobject-2.0', version: '>=' + glib_version)
-gio_dep = dependency('gio-2.0', version: '>=' + glib_version)
+if host_machine.system() == 'windows'
+  gio_dep = dependency('gio-2.0', version: '>=' + glib_version)
+else
+  gio_dep = dependency('gio-unix-2.0', version: '>=' + glib_version)
+endif
 glib_dep = declare_dependency(
   dependencies: [ glib_dep, gobject_dep, gio_dep ],
 )
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 4ab8832b37..d87425a64c 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -257,6 +257,7 @@
 @SRCDIR@src/util/virfirewall.c
 @SRCDIR@src/util/virfirewalld.c
 @SRCDIR@src/util/virfirmware.c
+@SRCDIR@src/util/virgdbus.c
 @SRCDIR@src/util/virhash.c
 @SRCDIR@src/util/virhook.c
 @SRCDIR@src/util/virhostcpu.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5842b8d23d..fea5a49e55 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2190,6 +2190,20 @@ virFirmwareParse;
 virFirmwareParseList;
 
 
+# util/virgdbus.h
+virGDBusCallMethod;
+virGDBusCallMethodWithFD;
+virGDBusCloseSystemBus;
+virGDBusErrorIsUnknownMethod;
+virGDBusGetSessionBus;
+virGDBusGetSystemBus;
+virGDBusHasSystemBus;
+virGDBusIsServiceEnabled;
+virGDBusIsServiceRegistered;
+virGDBusMessageIsSignal;
+virGDBusSetSharedBus;
+
+
 # util/virgettext.h
 virGettextInitialize;
 
diff --git a/src/util/meson.build b/src/util/meson.build
index bf556e7ae6..8a9dcac053 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -33,6 +33,7 @@ util_sources = [
   'virfirewall.c',
   'virfirewalld.c',
   'virfirmware.c',
+  'virgdbus.c',
   'virgettext.c',
   'virgic.c',
   'virhash.c',
diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c
new file mode 100644
index 00..535b19f0a4
--- /dev/null
+++ b/src/util/virgdbus.c
@@ -0,0 +1,425 @@
+/*
+ * virgdbus.c: helper for using GDBus
+ *
+ * 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 "virerror.h"
+#include "virlog.h"
+#include "virgdbus.h"
+#include "virthread.h"
+
+
+#define VIR_FROM_THIS VIR_FROM_DBUS
+
+VIR_LOG_INIT("util.dbus");
+
+
+static bool sharedBus = true;
+static GDBusConnection *systemBus;
+static GDBusConnection *sessionBus;
+static virOnceControl systemOnce = VIR_ONCE_CONTROL_INITIALIZER;
+static virOnceControl sessionOnce = VIR_ONCE_CONTROL_INITIALIZER;
+static GError *systemError;
+static GError *sessionError;
+
+
+void
+virGDBusSetSharedBus(bool shared)
+{
+sharedBus = shared;
+}
+
+
+static GDBusConnection *
+virGDBusBusInit(GBusType type, GError **error)
+{
+g_autofree char *address = NULL;
+
+if (sharedBus) {
+return g_bus_get_sync(type, NULL, error);
+} else {
+address = g_dbus_address_get_for_bus_sync(type, NULL, error);
+if (error)
+return NULL;
+return g_dbus_connection_new_for_address_sync(address,
+  
G_DBUS_CONNECTION_FLAGS_NONE,
+  NULL,
+  NULL,
+ 

[libvirt PATCH 12/14] src/remote/remote_daemon: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/remote/remote_daemon.c | 73 +++---
 1 file changed, 37 insertions(+), 36 deletions(-)

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 2ac4f6cd2e..9eb2c2bc0d 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -63,7 +63,7 @@
 
 #include "configmake.h"
 
-#include "virdbus.h"
+#include "virgdbus.h"
 
 VIR_LOG_INIT("daemon." DAEMON_NAME);
 
@@ -508,9 +508,8 @@ static void daemonInhibitCallback(bool inhibit, void 
*opaque)
 }
 
 
-#ifdef WITH_DBUS
-static DBusConnection *sessionBus;
-static DBusConnection *systemBus;
+static GDBusConnection *sessionBus;
+static GDBusConnection *systemBus;
 
 static void daemonStopWorker(void *opaque)
 {
@@ -538,41 +537,40 @@ static void daemonStop(virNetDaemonPtr dmn)
 }
 
 
-static DBusHandlerResult
-handleSessionMessageFunc(DBusConnection *connection G_GNUC_UNUSED,
- DBusMessage *message,
- void *opaque)
+static GDBusMessage *
+handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED,
+ GDBusMessage *message,
+ gboolean incoming G_GNUC_UNUSED,
+ gpointer opaque)
 {
 virNetDaemonPtr dmn = opaque;
 
 VIR_DEBUG("dmn=%p", dmn);
 
-if (dbus_message_is_signal(message,
-   DBUS_INTERFACE_LOCAL,
-   "Disconnected"))
+if (virGDBusMessageIsSignal(message,
+"org.freedesktop.DBus.Local",
+"Disconnected"))
 daemonStop(dmn);
 
-return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+return message;
 }
 
 
-static DBusHandlerResult
-handleSystemMessageFunc(DBusConnection *connection G_GNUC_UNUSED,
-DBusMessage *message,
-void *opaque)
+static void
+handleSystemMessageFunc(GDBusConnection *connection G_GNUC_UNUSED,
+const char *senderName G_GNUC_UNUSED,
+const char *objectPath G_GNUC_UNUSED,
+const char *interfaceName G_GNUC_UNUSED,
+const char *signalName G_GNUC_UNUSED,
+GVariant *parameters G_GNUC_UNUSED,
+gpointer opaque)
 {
 virNetDaemonPtr dmn = opaque;
 
 VIR_DEBUG("dmn=%p", dmn);
 
-if (dbus_message_is_signal(message,
-   "org.freedesktop.login1.Manager",
-   "PrepareForShutdown"))
-daemonStop(dmn);
-
-return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+daemonStop(dmn);
 }
-#endif
 
 
 static void daemonRunStateInit(void *opaque)
@@ -608,25 +606,28 @@ static void daemonRunStateInit(void *opaque)
 
 driversInitialized = true;
 
-#ifdef WITH_DBUS
 /* Tie the non-privileged daemons to the session/shutdown lifecycle */
 if (!virNetDaemonIsPrivileged(dmn)) {
 
-sessionBus = virDBusGetSessionBus();
+sessionBus = virGDBusGetSessionBus();
 if (sessionBus != NULL)
-dbus_connection_add_filter(sessionBus,
-   handleSessionMessageFunc, dmn, NULL);
+g_dbus_connection_add_filter(sessionBus,
+ handleSessionMessageFunc, dmn, NULL);
 
-systemBus = virDBusGetSystemBus();
-if (systemBus != NULL) {
-dbus_connection_add_filter(systemBus,
-   handleSystemMessageFunc, dmn, NULL);
-dbus_bus_add_match(systemBus,
-   "type='signal',sender='org.freedesktop.login1', 
interface='org.freedesktop.login1.Manager'",
-   NULL);
-}
+systemBus = virGDBusGetSystemBus();
+if (systemBus != NULL)
+g_dbus_connection_signal_subscribe(systemBus,
+   "org.freedesktop.login1",
+   
"org.freedesktop.login1.Manager",
+   "PrepareForShutdown",
+   NULL,
+   NULL,
+   G_DBUS_SIGNAL_FLAGS_NONE,
+   handleSystemMessageFunc,
+   dmn,
+   NULL);
 }
-#endif
+
 /* Only now accept clients from network */
 virNetDaemonUpdateServices(dmn, true);
  cleanup:
-- 
2.26.2

[libvirt PATCH 11/14] src/nwfilter/nwfilter_driver: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/nwfilter/meson.build   |   1 -
 src/nwfilter/nwfilter_driver.c | 127 -
 2 files changed, 46 insertions(+), 82 deletions(-)

diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build
index dcdc30f373..715282ee77 100644
--- a/src/nwfilter/meson.build
+++ b/src/nwfilter/meson.build
@@ -17,7 +17,6 @@ if conf.has('WITH_NWFILTER')
 ],
 dependencies: [
   access_dep,
-  dbus_dep,
   libnl_dep,
   libpcap_dep,
   src_dep,
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 77c4467816..1b8e3dbad3 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -24,7 +24,7 @@
 
 #include 
 
-#include "virdbus.h"
+#include "virgdbus.h"
 #include "virlog.h"
 
 #include "internal.h"
@@ -50,17 +50,6 @@
 
 VIR_LOG_INIT("nwfilter.nwfilter_driver");
 
-#define DBUS_RULE_FWD_NAMEOWNERCHANGED \
-"type='signal'" \
-",interface='"DBUS_INTERFACE_DBUS"'" \
-",member='NameOwnerChanged'" \
-",arg0='org.fedoraproject.FirewallD1'"
-
-#define DBUS_RULE_FWD_RELOADED \
-"type='signal'" \
-",interface='org.fedoraproject.FirewallD1'" \
-",member='Reloaded'"
-
 
 static virNWFilterDriverStatePtr driver;
 
@@ -79,36 +68,30 @@ static void nwfilterDriverUnlock(void)
 
 #ifdef WITH_FIREWALLD
 
-static DBusHandlerResult
-nwfilterFirewalldDBusFilter(DBusConnection *connection G_GNUC_UNUSED,
-DBusMessage *message,
-void *user_data G_GNUC_UNUSED)
+static void
+nwfilterFirewalldDBusSignalCallback(GDBusConnection *connection G_GNUC_UNUSED,
+const char *senderName G_GNUC_UNUSED,
+const char *objectPath G_GNUC_UNUSED,
+const char *interfaceName G_GNUC_UNUSED,
+const char *signalName G_GNUC_UNUSED,
+GVariant *parameters G_GNUC_UNUSED,
+gpointer user_data G_GNUC_UNUSED)
 {
-if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
-   "NameOwnerChanged") ||
-dbus_message_is_signal(message, "org.fedoraproject.FirewallD1",
-   "Reloaded")) {
-VIR_DEBUG("Reload in nwfilter_driver because of firewalld.");
-nwfilterStateReload();
-}
-
-return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+nwfilterStateReload();
 }
 
+static unsigned int restartID;
+static unsigned int reloadID;
+
 static void
 nwfilterDriverRemoveDBusMatches(void)
 {
-DBusConnection *sysbus;
+GDBusConnection *sysbus;
 
-sysbus = virDBusGetSystemBus();
+sysbus = virGDBusGetSystemBus();
 if (sysbus) {
-dbus_bus_remove_match(sysbus,
-  DBUS_RULE_FWD_NAMEOWNERCHANGED,
-  NULL);
-dbus_bus_remove_match(sysbus,
-  DBUS_RULE_FWD_RELOADED,
-  NULL);
-dbus_connection_remove_filter(sysbus, nwfilterFirewalldDBusFilter, 
NULL);
+g_dbus_connection_signal_unsubscribe(sysbus, restartID);
+g_dbus_connection_signal_unsubscribe(sysbus, reloadID);
 }
 }
 
@@ -117,33 +100,29 @@ nwfilterDriverRemoveDBusMatches(void)
  *
  * Startup DBus matches for monitoring the state of firewalld
  */
-static int
-nwfilterDriverInstallDBusMatches(DBusConnection *sysbus)
+static void
+nwfilterDriverInstallDBusMatches(GDBusConnection *sysbus)
 {
-int ret = 0;
-
-if (!sysbus) {
-ret = -1;
-} else {
-/* add matches for
- * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop
- * Reloaded on org.fedoraproject.FirewallD1 for firewalld reload
- */
-dbus_bus_add_match(sysbus,
-   DBUS_RULE_FWD_NAMEOWNERCHANGED,
-   NULL);
-dbus_bus_add_match(sysbus,
-   DBUS_RULE_FWD_RELOADED,
-   NULL);
-if (!dbus_connection_add_filter(sysbus, nwfilterFirewalldDBusFilter,
-NULL, NULL)) {
-VIR_WARN(("Adding a filter to the DBus connection failed"));
-nwfilterDriverRemoveDBusMatches();
-ret =  -1;
-}
-}
-
-return ret;
+restartID = g_dbus_connection_signal_subscribe(sysbus,
+   NULL,
+   "org.freedesktop.DBus",
+   "NameOwnerChanged",
+   NULL,
+   
"org.fedoraproject.FirewallD1",
+   G_DBUS_SIGNAL_FLAGS_NONE,
+   

[libvirt PATCH 14/14] drop libdbus from libvirt

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 libvirt.spec.in |3 -
 meson.build |   27 +-
 meson_options.txt   |1 -
 po/POTFILES.in  |1 -
 src/libvirt_private.syms|   17 -
 src/libvirt_probes.d|6 -
 src/meson.build |1 -
 src/remote/remote_daemon_dispatch.c |1 -
 src/util/meson.build|2 -
 src/util/virdbus.c  | 1871 ---
 src/util/virdbus.h  |   76 --
 src/util/virdbuspriv.h  |   56 -
 tests/meson.build   |   13 -
 tests/virdbusmock.c |   62 -
 tests/virdbustest.c |  728 ---
 15 files changed, 3 insertions(+), 2862 deletions(-)
 delete mode 100644 src/util/virdbus.c
 delete mode 100644 src/util/virdbus.h
 delete mode 100644 src/util/virdbuspriv.h
 delete mode 100644 tests/virdbusmock.c
 delete mode 100644 tests/virdbustest.c

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 62b401bd08..264932e6d5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -374,9 +374,6 @@ BuildRequires: util-linux
 # For showmount in FS driver (netfs discovery)
 BuildRequires: nfs-utils
 
-# Communication with the firewall and polkit daemons use DBus
-BuildRequires: dbus-devel
-
 # Fedora build root suckage
 BuildRequires: gawk
 
diff --git a/meson.build b/meson.build
index b5b1223227..24535a403c 100644
--- a/meson.build
+++ b/meson.build
@@ -1019,22 +1019,6 @@ if curl_dep.found()
   conf.set('WITH_CURL', 1)
 endif
 
-dbus_version = '1.0.0'
-dbus_dep = dependency('dbus-1', version: '>=' + dbus_version, required: 
get_option('dbus'))
-if dbus_dep.found()
-  conf.set('WITH_DBUS', 1)
-
-  function = 'dbus_watch_get_unix_fd'
-  if cc.has_function(function, dependencies: dbus_dep)
-conf.set('WITH_@0@'.format(function.to_upper()), 1)
-  endif
-
-  type = 'DBusBasicValue'
-  if cc.has_type(type, dependencies: dbus_dep, prefix: '#include 
')
-conf.set('WITH_@0@'.format(type.to_upper()), 1)
-  endif
-endif
-
 devmapper_version = '1.0.0'
 devmapper_dep = dependency('devmapper', version: '>=' + devmapper_version, 
required: false)
 if not devmapper_dep.found()
@@ -1536,10 +1520,8 @@ if bash_completion_dep.found()
 endif
 
 if host_machine.system() != 'freebsd'
-  if not get_option('firewalld').disabled() and dbus_dep.found()
+  if not get_option('firewalld').disabled()
 conf.set('WITH_FIREWALLD', 1)
-  elif get_option('firewalld').enabled()
-error('You must have dbus enabled for firewalld support')
   endif
 endif
 
@@ -1553,10 +1535,8 @@ if conf.has('WITH_MACVTAP') and not 
conf.has('WITH_LIBNL')
   error('libnl3-devel is required for macvtap support')
 endif
 
-if (pkcheck_prog.found() or get_option('polkit').enabled()) and 
dbus_dep.found()
+if (pkcheck_prog.found() or get_option('polkit').enabled())
   conf.set('WITH_POLKIT', 1)
-elif get_option('polkit').enabled()
-  error('You must install dbus to compile libvirt with polkit-1')
 endif
 
 if udev_dep.found() and not pciaccess_dep.found()
@@ -2203,7 +2183,7 @@ endif
 
 if not get_option('pm_utils').disabled()
   use_pm_utils = true
-  if dbus_dep.found() and init_script == 'systemd'
+  if init_script == 'systemd'
 use_pm_utils = false
   endif
 
@@ -2423,7 +2403,6 @@ libs_summary = {
   'blkid': blkid_dep.found(),
   'capng': capng_dep.found(),
   'curl': curl_dep.found(),
-  'dbus': dbus_dep.found(),
   'dlopen': dlopen_dep.found(),
   'firewalld': conf.has('WITH_FIREWALLD'),
   'firewalld-zone': conf.has('WITH_FIREWALLD_ZONE'),
diff --git a/meson_options.txt b/meson_options.txt
index c8886e1430..f92c80553c 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -18,7 +18,6 @@ option('bash_completion_dir', type: 'string', value: '', 
description: 'directory
 option('blkid', type: 'feature', value: 'auto', description: 'blkid support')
 option('capng', type: 'feature', value: 'auto', description: 'cap-ng support')
 option('curl', type: 'feature', value: 'auto', description: 'curl support')
-option('dbus', type: 'feature', value: 'auto', description: 'dbus-1 support')
 option('firewalld', type: 'feature', value: 'auto', description: 'firewalld 
support')
 option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether 
to install firewalld libvirt zone')
 option('fuse', type: 'feature', value: 'auto', description: 'fuse support')
diff --git a/po/POTFILES.in b/po/POTFILES.in
index d87425a64c..54f78e7861 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -243,7 +243,6 @@
 @SRCDIR@src/util/virconf.c
 @SRCDIR@src/util/vircrypto.c
 @SRCDIR@src/util/virdaemon.c
-@SRCDIR@src/util/virdbus.c
 @SRCDIR@src/util/virdevmapper.c
 @SRCDIR@src/util/virdnsmasq.c
 @SRCDIR@src/util/virerror.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fea5a49e55..bdbe3431b8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1948,23 +1948,6 @@ 

[libvirt PATCH 06/14] src/util/virpolkit: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/util/virpolkit.c  | 115 +-
 tests/meson.build |  11 ++--
 tests/virpolkittest.c | 112 +++-
 3 files changed, 104 insertions(+), 134 deletions(-)

diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index 1570d667ee..2ad00fd206 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -28,7 +28,7 @@
 #include "virstring.h"
 #include "virprocess.h"
 #include "viralloc.h"
-#include "virdbus.h"
+#include "virgdbus.h"
 #include "virfile.h"
 #include "virutil.h"
 
@@ -63,80 +63,81 @@ int virPolkitCheckAuth(const char *actionid,
const char **details,
bool allowInteraction)
 {
-DBusConnection *sysbus;
-DBusMessage *reply = NULL;
-char **retdetails = NULL;
-size_t nretdetails = 0;
-bool is_authorized;
-bool is_challenge;
+GDBusConnection *sysbus;
+GVariantBuilder builder;
+GVariant *gprocess = NULL;
+GVariant *gdetails = NULL;
+g_autoptr(GVariant) message = NULL;
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(GVariantIter) iter = NULL;
+char *retkey;
+char *retval;
+gboolean is_authorized;
+gboolean is_challenge;
 bool is_dismissed = false;
 size_t i;
-int ret = -1;
 
-if (!(sysbus = virDBusGetSystemBus()))
-goto cleanup;
+if (!(sysbus = virGDBusGetSystemBus()))
+return -1;
 
 VIR_INFO("Checking PID %lld running as %d",
  (long long) pid, uid);
 
-if (virDBusCallMethod(sysbus,
-  ,
-  NULL,
-  "org.freedesktop.PolicyKit1",
-  "/org/freedesktop/PolicyKit1/Authority",
-  "org.freedesktop.PolicyKit1.Authority",
-  "CheckAuthorization",
-  "(sa{sv})sa&{ss}us",
-  "unix-process",
-  3,
-  "pid", "u", (unsigned int)pid,
-  "start-time", "t", startTime,
-  "uid", "i", (int)uid,
-  actionid,
-  virStringListLength(details) / 2,
-  details,
-  allowInteraction,
-  "" /* cancellation ID */) < 0)
-goto cleanup;
+g_variant_builder_init(, G_VARIANT_TYPE("a{sv}"));
+g_variant_builder_add(, "{sv}", "pid", g_variant_new_uint32(pid));
+g_variant_builder_add(, "{sv}", "start-time", 
g_variant_new_uint64(startTime));
+g_variant_builder_add(, "{sv}", "uid", g_variant_new_int32(uid));
+gprocess = g_variant_builder_end();
 
-if (virDBusMessageDecode(reply,
- "(bba&{ss})",
- _authorized,
- _challenge,
- ,
- ) < 0)
-goto cleanup;
+g_variant_builder_init(, G_VARIANT_TYPE("a{ss}"));
+for (i = 0; i < virStringListLength(details); i += 2)
+g_variant_builder_add(, "{ss}", details[i], details[i + 1]);
+gdetails = g_variant_builder_end();
 
-for (i = 0; i < (nretdetails / 2); i++) {
-if (STREQ(retdetails[(i * 2)], "polkit.dismissed") &&
-STREQ(retdetails[(i * 2) + 1], "true"))
+message = g_variant_new("((s@a{sv})s@a{ss}us)",
+"unix-process",
+gprocess,
+actionid,
+gdetails,
+allowInteraction,
+"" /* cancellation ID */);
+
+if (virGDBusCallMethod(sysbus,
+   ,
+   NULL,
+   "org.freedesktop.PolicyKit1",
+   "/org/freedesktop/PolicyKit1/Authority",
+   "org.freedesktop.PolicyKit1.Authority",
+   "CheckAuthorization",
+   message) < 0)
+return -1;
+
+g_variant_get(reply, "((bba{ss}))", _authorized, _challenge, );
+
+while (g_variant_iter_loop(iter, "{ss}", , )) {
+if (STREQ(retkey, "polkit.dismissed") && STREQ(retval, "true"))
 is_dismissed = true;
 }
 
 VIR_DEBUG("is auth %d  is challenge %d",
   is_authorized, is_challenge);
 
-if (is_authorized) {
-ret = 0;
+if (is_authorized)
+return 0;
+
+if (is_dismissed) {
+virReportError(VIR_ERR_AUTH_CANCELLED, "%s",
+   _("user cancelled authentication process"));
+} else if (is_challenge) {
+virReportError(VIR_ERR_AUTH_UNAVAILABLE,
+   _("no polkit agent available to authenticate action 
'%s'"),
+   actionid);
 } else {
-ret = -2;
-if (is_dismissed)
-

[libvirt PATCH 04/14] tests/virmock: extend number of arguments

[Pavel Hrdina]

Rewrite to use GLib DBus instead of libdbus will introduce function with
large number of arguments that we will have to mock for our tests so we
need to extend the number of arguments for our macros.

Signed-off-by: Pavel Hrdina 
---
 tests/virmock.h | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/virmock.h b/tests/virmock.h
index a812cfe08b..dea5feb80f 100644
--- a/tests/virmock.h
+++ b/tests/virmock.h
@@ -27,8 +27,8 @@
 
 #include "internal.h"
 
-#define VIR_MOCK_COUNT_ARGS(...) VIR_MOCK_ARG21(__VA_ARGS__, 20, 19, 18, 17, 
16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1)
-#define VIR_MOCK_ARG21(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, 
_14, _15, _16, _17, _18, _19, _20, _21, ...) _21
+#define VIR_MOCK_COUNT_ARGS(...) VIR_MOCK_ARG27(__VA_ARGS__, 26, 25, 24, 23, 
22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1)
+#define VIR_MOCK_ARG27(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, 
_14, _15, _16, _17, _18, _19, _20, _21, _22, _23, _24, _25, _26, _27, ...) _27
 #define VIR_MOCK_ARG_PASTE(a, b, ...) a##b(__VA_ARGS__)
 
 #define VIR_MOCK_ARGNAME(a, b) b
@@ -56,6 +56,12 @@
 #define VIR_MOCK_GET_ARG19(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, l), z(m, n), z(o, 
p), z(q, r)
 #define VIR_MOCK_GET_ARG20(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, l), z(m, n), 
z(o, p), z(q, r), z(s, t)
 #define VIR_MOCK_GET_ARG21(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, l), z(m, n), 
z(o, p), z(q, r), z(s, t)
+#define VIR_MOCK_GET_ARG22(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u, v) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, l), z(m, 
n), z(o, p), z(q, r), z(s, t), z(u, v)
+#define VIR_MOCK_GET_ARG23(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u, v, w) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, l), 
z(m, n), z(o, p), z(q, r), z(s, t), z(u, v)
+#define VIR_MOCK_GET_ARG24(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u, v, w, x) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, l), 
z(m, n), z(o, p), z(q, r), z(s, t), z(u, v), z(w, x)
+#define VIR_MOCK_GET_ARG25(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u, v, w, x, y) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), z(k, 
l), z(m, n), z(o, p), z(q, r), z(s, t), z(u, v), z(w, x)
+#define VIR_MOCK_GET_ARG26(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u, v, w, x, y, aa) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, j), 
z(k, l), z(m, n), z(o, p), z(q, r), z(s, t), z(u, v), z(w, x), z(y, aa)
+#define VIR_MOCK_GET_ARG27(z, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, 
q, r, s, t, u, v, w, x, y, aa, ab) z(a, b),  z(c, d), z(e, f), z(g, h), z(i, 
j), z(k, l), z(m, n), z(o, p), z(q, r), z(s, t), z(u, v), z(w, x), z(y, aa)
 
 
 #define VIR_MOCK_ARGNAMES_EXPAND(a, b, ...) VIR_MOCK_ARG_PASTE(a, b, 
__VA_ARGS__)
-- 
2.26.2

[libvirt PATCH 09/14] src/lxc/lxc_controller: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/lxc/lxc_controller.c | 6 +++---
 src/lxc/meson.build  | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 37a28ac2f3..c3cf485e2c 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -54,13 +54,13 @@
 #include "virnetdevveth.h"
 #include "viralloc.h"
 #include "virfile.h"
+#include "virgdbus.h"
 #include "virpidfile.h"
 #include "vircommand.h"
 #include "virhostcpu.h"
 #include "virrandom.h"
 #include "virprocess.h"
 #include "virnuma.h"
-#include "virdbus.h"
 #include "rpc/virnetdaemon.h"
 #include "virstring.h"
 #include "virgettext.h"
@@ -2430,7 +2430,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
  * of LXC instance, since dbus-daemon is limited to
  * only a few 100 connections by default
  */
-virDBusCloseSystemBus();
+virGDBusCloseSystemBus();
 
 rc = virLXCControllerMain(ctrl);
 
@@ -2612,7 +2612,7 @@ int main(int argc, char *argv[])
 
 virEventRegisterDefaultImpl();
 
-virDBusSetSharedBus(false);
+virGDBusSetSharedBus(false);
 
 if (!(ctrl = virLXCControllerNew(name)))
 goto cleanup;
diff --git a/src/lxc/meson.build b/src/lxc/meson.build
index 2810da7604..f8e2a8852a 100644
--- a/src/lxc/meson.build
+++ b/src/lxc/meson.build
@@ -139,7 +139,6 @@ if conf.has('WITH_LXC')
 'deps': [
   blkid_dep,
   capng_dep,
-  dbus_dep,
   fuse_dep,
   libnl_dep,
   rpc_dep,
-- 
2.26.2

[libvirt PATCH 01/14] remove HAL node device driver

[Pavel Hrdina]

There was one attempt a year ago done by me to drop HAL [1] but it was
never resolved. There was another time when Dan suggested to drop HAL
driver [2] but it was decided to keep it around in case device
assignment will be implemented for FreeBSD and the fact that
virt-manager uses node device driver [3].

I checked git history and code and it doesn't look like bhyve supports
device assignment so from that POV it should not block removing HAL.

The argument about virt-manager is not strong as well because libvirt
installed from FreeBSD packages doesn't have HAL support so it will not
affect these users as well [4].

The only users affected by this change would be the ones compiling
libvirt from GIT on FreeBSD.

I looked into alternatives and there is libudev-devd package on FreeBSD
but unfortunately it doesn't work as it doesn't list any devices when
used with libvirt. It provides libudev APIs using devd.

I also looked into devd directly and it provides some APIs but there are
no APIs for device monitoring and events so that would have to be
somehow done by libvirt.

Main motivation for dropping HAL support is to replace libdbus with GLib
dbus implementation and it cannot be done with HAL driver present in
libvirt because HAL APIs heavily depends on symbols provided by libdbus.

[1] 
[2] 
[3] 
[4] 

Signed-off-by: Pavel Hrdina 
---
 meson.build  |   9 +-
 meson_options.txt|   1 -
 po/POTFILES.in   |   1 -
 src/node_device/meson.build  |   5 -
 src/node_device/node_device_driver.c |  10 +-
 src/node_device/node_device_driver.h |   5 -
 src/node_device/node_device_hal.c| 843 ---
 src/node_device/node_device_hal.h|  22 -
 8 files changed, 3 insertions(+), 893 deletions(-)
 delete mode 100644 src/node_device/node_device_hal.c
 delete mode 100644 src/node_device/node_device_hal.h

diff --git a/meson.build b/meson.build
index 195d7cd784..3d15b4ee34 100644
--- a/meson.build
+++ b/meson.build
@@ -1077,12 +1077,6 @@ glusterfs_dep = dependency('glusterfs-api', version: 
'>=' + glusterfs_version, r
 gnutls_version = '3.2.0'
 gnutls_dep = dependency('gnutls', version: '>=' + gnutls_version)
 
-hal_version = '0.5.0'
-hal_dep = dependency('hal', version: '>=' + hal_version, required: 
get_option('hal'))
-if hal_dep.found()
-  conf.set('WITH_HAL', 1)
-endif
-
 # Check for BSD kvm (kernel memory interface)
 if host_machine.system() == 'freebsd'
   kvm_dep = cc.find_library('kvm')
@@ -1728,7 +1722,7 @@ if not get_option('driver_network').disabled() and 
conf.has('WITH_LIBVIRTD') and
   conf.set('WITH_NETWORK', 1)
 endif
 
-if hal_dep.found() or udev_dep.found()
+if udev_dep.found()
   conf.set('WITH_NODE_DEVICES', 1)
 endif
 
@@ -2433,7 +2427,6 @@ libs_summary = {
   'glib_dep': glib_dep.found(),
   'glusterfs': glusterfs_dep.found(),
   'gnutls': gnutls_dep.found(),
-  'hal': hal_dep.found(),
   'libiscsi': libiscsi_dep.found(),
   'libnl': libnl_dep.found(),
   'libpcap': libpcap_dep.found(),
diff --git a/meson_options.txt b/meson_options.txt
index 7838630c1e..c8886e1430 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -23,7 +23,6 @@ option('firewalld', type: 'feature', value: 'auto', 
description: 'firewalld supp
 option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether 
to install firewalld libvirt zone')
 option('fuse', type: 'feature', value: 'auto', description: 'fuse support')
 option('glusterfs', type: 'feature', value: 'auto', description: 'glusterfs 
support')
-option('hal', type: 'feature', value: 'auto', description: 'hal support')
 option('libiscsi', type: 'feature', value: 'auto', description: 'libiscsi 
support')
 option('libpcap', type: 'feature', value: 'auto', description: 'libpcap 
support')
 option('libssh', type: 'feature', value: 'auto', description: 'libssh support')
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 471af30b89..4ab8832b37 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -130,7 +130,6 @@
 @SRCDIR@src/network/bridge_driver_linux.c
 @SRCDIR@src/network/leaseshelper.c
 @SRCDIR@src/node_device/node_device_driver.c
-@SRCDIR@src/node_device/node_device_hal.c
 @SRCDIR@src/node_device/node_device_udev.c
 @SRCDIR@src/nwfilter/nwfilter_dhcpsnoop.c
 @SRCDIR@src/nwfilter/nwfilter_driver.c
diff --git a/src/node_device/meson.build b/src/node_device/meson.build
index 5953c6b8ed..c4e4c3906b 100644
--- a/src/node_device/meson.build
+++ b/src/node_device/meson.build
@@ -4,10 +4,6 @@ node_device_driver_sources = [
 
 stateful_driver_source_files += files(node_device_driver_sources)
 
-if conf.has('WITH_HAL')
-  node_device_driver_sources += 'node_device_hal.c'
-endif
-
 if 

[libvirt PATCH 10/14] src/network/bridge_driver: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/network/bridge_driver.c | 81 ++---
 src/network/meson.build |  2 -
 2 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 87d7acab06..5d9b9eaa4f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -59,7 +59,7 @@
 #include "virnetdevtap.h"
 #include "virnetdevvportprofile.h"
 #include "virpci.h"
-#include "virdbus.h"
+#include "virgdbus.h"
 #include "virfile.h"
 #include "virstring.h"
 #include "viraccessapicheck.h"
@@ -638,33 +638,29 @@ networkAutostartConfig(virNetworkObjPtr obj,
 
 
 #ifdef WITH_FIREWALLD
-static DBusHandlerResult
-firewalld_dbus_filter_bridge(DBusConnection *connection G_GNUC_UNUSED,
- DBusMessage *message,
- void *user_data)
+static void
+firewalld_dbus_signal_callback(GDBusConnection *connection G_GNUC_UNUSED,
+   const char *senderName G_GNUC_UNUSED,
+   const char *objectPath G_GNUC_UNUSED,
+   const char *interfaceName,
+   const char *signalName,
+   GVariant *parameters,
+   gpointer user_data)
 {
 virNetworkDriverStatePtr driver = user_data;
 bool reload = false;
 
-if (dbus_message_is_signal(message,
-   "org.fedoraproject.FirewallD1", "Reloaded")) {
+if (STREQ(interfaceName, "org.fedoraproject.FirewallD1") &&
+STREQ(signalName, "Reloaded")) {
 reload = true;
+} else if (STREQ(interfaceName, "org.freedesktop.DBus") &&
+   STREQ(signalName, "NameOwnerChanged")) {
+char *name = NULL;
+char *old_owner = NULL;
+char *new_owner = NULL;
 
-} else if (dbus_message_is_signal(message,
-  DBUS_INTERFACE_DBUS, 
"NameOwnerChanged")) {
+g_variant_get(parameters, "()", , _owner, _owner);
 
-g_autofree char *name = NULL;
-g_autofree char *old_owner = NULL;
-g_autofree char *new_owner = NULL;
-
-if (virDBusMessageDecode(message, "sss", , _owner, 
_owner) < 0) {
-VIR_WARN("Failed to decode DBus NameOwnerChanged message");
-return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-}
-/*
- * if new_owner is empty, firewalld is shutting down. If it is
- * non-empty, then it is starting
- */
 if (new_owner && *new_owner)
 reload = true;
 }
@@ -673,8 +669,6 @@ firewalld_dbus_filter_bridge(DBusConnection *connection 
G_GNUC_UNUSED,
 VIR_DEBUG("Reload in bridge_driver because of firewalld.");
 networkReloadFirewallRules(driver, false, true);
 }
-
-return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 #endif
 
@@ -694,7 +688,7 @@ networkStateInitialize(bool privileged,
 g_autofree char *rundir = NULL;
 bool autostart = true;
 #ifdef WITH_FIREWALLD
-DBusConnection *sysbus = NULL;
+GDBusConnection *sysbus = NULL;
 #endif
 
 if (root != NULL) {
@@ -793,27 +787,30 @@ networkStateInitialize(bool privileged,
 network_driver->networkEventState = virObjectEventStateNew();
 
 #ifdef WITH_FIREWALLD
-if (!(sysbus = virDBusGetSystemBus())) {
+if (!(sysbus = virGDBusGetSystemBus())) {
 VIR_WARN("DBus not available, disabling firewalld support "
  "in bridge_network_driver: %s", virGetLastErrorMessage());
 } else {
-/* add matches for
- * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop
- * Reloaded on org.fedoraproject.FirewallD1 for firewalld reload
- */
-dbus_bus_add_match(sysbus,
-   "type='signal'"
-   ",interface='"DBUS_INTERFACE_DBUS"'"
-   ",member='NameOwnerChanged'"
-   ",arg0='org.fedoraproject.FirewallD1'",
-   NULL);
-dbus_bus_add_match(sysbus,
-   "type='signal'"
-   ",interface='org.fedoraproject.FirewallD1'"
-   ",member='Reloaded'",
-   NULL);
-dbus_connection_add_filter(sysbus, firewalld_dbus_filter_bridge,
-   network_driver, NULL);
+g_dbus_connection_signal_subscribe(sysbus,
+   NULL,
+   "org.freedesktop.DBus",
+   "NameOwnerChanged",
+   NULL,
+   "org.fedoraproject.FirewallD1",
+   G_DBUS_SIGNAL_FLAGS_NONE,
+   firewalld_dbus_signal_callback,
+   

[libvirt PATCH 13/14] src/rpc/virnetdaemon: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/rpc/meson.build|   1 -
 src/rpc/virnetdaemon.c | 115 +
 2 files changed, 35 insertions(+), 81 deletions(-)

diff --git a/src/rpc/meson.build b/src/rpc/meson.build
index e249b9d534..cc1424140a 100644
--- a/src/rpc/meson.build
+++ b/src/rpc/meson.build
@@ -98,7 +98,6 @@ virt_rpc_server_lib = static_library(
 rpc_gen_headers,
   ],
   dependencies: [
-dbus_dep,
 sasl_dep,
 src_dep,
 xdr_dep,
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index b6c32ee277..12d4d9bf87 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -32,7 +32,7 @@
 #include "virutil.h"
 #include "virfile.h"
 #include "virnetserver.h"
-#include "virdbus.h"
+#include "virgdbus.h"
 #include "virhash.h"
 #include "virstring.h"
 #include "virsystemd.h"
@@ -78,7 +78,6 @@ struct _virNetDaemon {
 
 unsigned int autoShutdownTimeout;
 size_t autoShutdownInhibitions;
-bool autoShutdownCallingInhibit;
 int autoShutdownInhibitFd;
 };
 
@@ -459,53 +458,7 @@ virNetDaemonAutoShutdown(virNetDaemonPtr dmn,
 }
 
 
-#if defined(WITH_DBUS) && defined(DBUS_TYPE_UNIX_FD)
-static void
-virNetDaemonGotInhibitReplyLocked(DBusPendingCall *pending,
-  virNetDaemonPtr dmn)
-{
-DBusMessage *reply;
-int fd;
-
-dmn->autoShutdownCallingInhibit = false;
-
-VIR_DEBUG("dmn=%p", dmn);
-
-reply = dbus_pending_call_steal_reply(pending);
-if (reply == NULL)
-goto cleanup;
-
-if (dbus_message_get_args(reply, NULL,
-  DBUS_TYPE_UNIX_FD, ,
-  DBUS_TYPE_INVALID)) {
-if (dmn->autoShutdownInhibitions) {
-dmn->autoShutdownInhibitFd = fd;
-VIR_DEBUG("Got inhibit FD %d", fd);
-} else {
-/* We stopped the last VM since we made the inhibit call */
-VIR_DEBUG("Closing inhibit FD %d", fd);
-VIR_FORCE_CLOSE(fd);
-}
-}
-virDBusMessageUnref(reply);
-
- cleanup:
-dbus_pending_call_unref(pending);
-}
-
-
-static void
-virNetDaemonGotInhibitReply(DBusPendingCall *pending,
-void *opaque)
-{
-virNetDaemonPtr dmn = opaque;
-
-virObjectLock(dmn);
-virNetDaemonGotInhibitReplyLocked(pending, dmn);
-virObjectUnlock(dmn);
-}
-
-
+#ifdef G_OS_UNIX
 /* As per: https://www.freedesktop.org/wiki/Software/systemd/inhibit */
 static void
 virNetDaemonCallInhibit(virNetDaemonPtr dmn,
@@ -514,9 +467,12 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
 const char *why,
 const char *mode)
 {
-DBusMessage *message;
-DBusPendingCall *pendingReply = NULL;
-DBusConnection *systemBus;
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(GUnixFDList) replyFD = NULL;
+GVariant *message = NULL;
+GDBusConnection *systemBus;
+int fd;
+int rc;
 
 VIR_DEBUG("dmn=%p what=%s who=%s why=%s mode=%s",
   dmn, NULLSTR(what), NULLSTR(who), NULLSTR(why), NULLSTR(mode));
@@ -524,41 +480,40 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
 if (virSystemdHasLogind() < 0)
 return;
 
-if (!(systemBus = virDBusGetSystemBus()))
+if (!(systemBus = virGDBusGetSystemBus()))
 return;
 
-/* Only one outstanding call at a time */
-if (dmn->autoShutdownCallingInhibit)
+message = g_variant_new("()", what, who, why, mode);
+
+rc = virGDBusCallMethodWithFD(systemBus,
+  ,
+  ,
+  NULL,
+  "org.freedesktop.login1",
+  "/org/freedesktop/login1",
+  "org.freedesktop.login1.Manager",
+  "Inhibit",
+  message,
+  NULL);
+
+if (rc < 0)
 return;
 
-message = dbus_message_new_method_call("org.freedesktop.login1",
-   "/org/freedesktop/login1",
-   "org.freedesktop.login1.Manager",
-   "Inhibit");
-if (message == NULL)
+if (g_unix_fd_list_get_length(replyFD) <= 0)
 return;
 
-dbus_message_append_args(message,
- DBUS_TYPE_STRING, ,
- DBUS_TYPE_STRING, ,
- DBUS_TYPE_STRING, ,
- DBUS_TYPE_STRING, ,
- DBUS_TYPE_INVALID);
+fd = g_unix_fd_list_get(replyFD, 0, NULL);
+if (fd < 0)
+return;
 
-if (dbus_connection_send_with_reply(systemBus, message,
-,
-25 * 1000) &&
-pendingReply) {
-if (dbus_pending_call_get_completed(pendingReply)) {
-

[libvirt PATCH 02/14] tests: mock libdbus in networkxml2firewalltest

[Pavel Hrdina]

This test calls into src/util/virfirewalld.c where it uses DBus to
figure out if firewalld is registered. Without the mock it luckily
fails and the test works correctly.

To isolate the tests from host environment we should mock the DBus
calls.

Signed-off-by: Pavel Hrdina 
---
 tests/meson.build   |  2 +-
 tests/networkxml2firewalltest.c | 25 +
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/tests/meson.build b/tests/meson.build
index 0a204c46e4..f0f3d8c1ef 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -404,7 +404,7 @@ endif
 if conf.has('WITH_NETWORK')
   tests += [
 { 'name': 'networkxml2conftest', 'link_with': [ network_driver_impl ] },
-{ 'name': 'networkxml2firewalltest', 'link_with': [ network_driver_impl ] 
},
+{ 'name': 'networkxml2firewalltest', 'link_with': [ network_driver_impl ], 
'deps': [ dbus_dep ] },
 { 'name': 'networkxml2xmltest', 'link_with': [ network_driver_impl ] },
   ]
 endif
diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
index 29e7d8bc38..80e2d6a035 100644
--- a/tests/networkxml2firewalltest.c
+++ b/tests/networkxml2firewalltest.c
@@ -26,8 +26,13 @@
 
 #if defined (__linux__)
 
+# if WITH_DBUS
+#  include 
+# endif
+
 # include "network/bridge_driver_platform.h"
 # include "virbuffer.h"
+# include "virmock.h"
 
 # define LIBVIRT_VIRFIREWALLPRIV_H_ALLOW
 # include "virfirewallpriv.h"
@@ -43,6 +48,22 @@
 #  error "test case not ported to this platform"
 # endif
 
+# if WITH_DBUS
+VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
+   DBusMessage *,
+   DBusConnection *, connection,
+   DBusMessage *, message,
+   int, timeout_milliseconds,
+   DBusError *, error)
+{
+VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
+
+dbus_set_error_const(error, "org.freedesktop.error", "dbus is disabled");
+
+return NULL;
+}
+# endif
+
 static void
 testCommandDryRun(const char *const*args G_GNUC_UNUSED,
   const char *const*env G_GNUC_UNUSED,
@@ -176,7 +197,11 @@ mymain(void)
 return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
+# if WITH_DBUS
+VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdbus"))
+# else
 VIR_TEST_MAIN(mymain)
+# endif
 
 #else /* ! defined (__linux__) */
 
-- 
2.26.2

[libvirt PATCH 08/14] src/util/virsystemd: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/util/virsystemd.c  | 371 +
 tests/meson.build  |   2 +-
 tests/virsystemdtest.c | 173 +++
 3 files changed, 254 insertions(+), 292 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 0ae896f4a3..32c830c002 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -26,7 +26,7 @@
 
 #include "virsystemd.h"
 #include "virbuffer.h"
-#include "virdbus.h"
+#include "virgdbus.h"
 #include "virstring.h"
 #include "viralloc.h"
 #include "virutil.h"
@@ -157,13 +157,13 @@ virSystemdHasMachined(void)
 if (val != -1)
 return val;
 
-if ((ret = virDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) {
+if ((ret = virGDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) {
 if (ret == -2)
 g_atomic_int_set(, -2);
 return ret;
 }
 
-if ((ret = virDBusIsServiceRegistered("org.freedesktop.systemd1")) == -1)
+if ((ret = virGDBusIsServiceRegistered("org.freedesktop.systemd1")) == -1)
 return ret;
 g_atomic_int_set(, ret);
 return ret;
@@ -179,14 +179,14 @@ virSystemdHasLogind(void)
 if (val != -1)
 return val;
 
-ret = virDBusIsServiceEnabled("org.freedesktop.login1");
+ret = virGDBusIsServiceEnabled("org.freedesktop.login1");
 if (ret < 0) {
 if (ret == -2)
 g_atomic_int_set(, -2);
 return ret;
 }
 
-if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) == -1)
+if ((ret = virGDBusIsServiceRegistered("org.freedesktop.login1")) == -1)
 return ret;
 
 g_atomic_int_set(, ret);
@@ -197,53 +197,59 @@ virSystemdHasLogind(void)
 char *
 virSystemdGetMachineNameByPID(pid_t pid)
 {
-DBusConnection *conn;
-DBusMessage *reply = NULL;
-char *name = NULL, *object = NULL;
+GDBusConnection *conn;
+g_autoptr(GVariant) message = NULL;
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(GVariant) gvar = NULL;
+g_autofree char *object = NULL;
+char *name = NULL;
 
 if (virSystemdHasMachined() < 0)
-goto cleanup;
+return NULL;
 
-if (!(conn = virDBusGetSystemBus()))
-goto cleanup;
+if (!(conn = virGDBusGetSystemBus()))
+return NULL;
 
-if (virDBusCallMethod(conn, , NULL,
-  "org.freedesktop.machine1",
-  "/org/freedesktop/machine1",
-  "org.freedesktop.machine1.Manager",
-  "GetMachineByPID",
-  "u", pid) < 0)
-goto cleanup;
+message = g_variant_new("(u)", pid);
 
-if (virDBusMessageDecode(reply, "o", ) < 0)
-goto cleanup;
+if (virGDBusCallMethod(conn,
+   ,
+   NULL,
+   "org.freedesktop.machine1",
+   "/org/freedesktop/machine1",
+   "org.freedesktop.machine1.Manager",
+   "GetMachineByPID",
+   message) < 0)
+return NULL;
 
-virDBusMessageUnref(reply);
+g_variant_get(reply, "(o)", );
+
+g_variant_unref(reply);
 reply = NULL;
 
 VIR_DEBUG("Domain with pid %lld has object path '%s'",
   (long long) pid, object);
 
-if (virDBusCallMethod(conn, , NULL,
-  "org.freedesktop.machine1",
-  object,
-  "org.freedesktop.DBus.Properties",
-  "Get",
-  "ss",
-  "org.freedesktop.machine1.Machine",
-  "Name") < 0)
-goto cleanup;
+g_variant_unref(message);
+message = g_variant_new("(ss)",
+"org.freedesktop.machine1.Machine", "Name");
 
-if (virDBusMessageDecode(reply, "v", "s", ) < 0)
-goto cleanup;
+if (virGDBusCallMethod(conn,
+   ,
+   NULL,
+   "org.freedesktop.machine1",
+   object,
+   "org.freedesktop.DBus.Properties",
+   "Get",
+   message) < 0)
+return NULL;
+
+g_variant_get(reply, "(v)", );
+g_variant_get(gvar, "s", );
 
 VIR_DEBUG("Domain with pid %lld has machine name '%s'",
   (long long) pid, name);
 
- cleanup:
-VIR_FREE(object);
-virDBusMessageUnref(reply);
-
 return name;
 }
 
@@ -274,26 +280,28 @@ int virSystemdCreateMachine(const char *name,
 const char *partition,
 unsigned int maxthreads)
 {
-int ret;
-DBusConnection *conn;
-char *creatorname = NULL;
-char *slicename = NULL;
-char *scopename = NULL;
+int rc;
+GDBusConnection *conn;
+GVariant *guuid;
+GVariant *gnicindexes;
+GVariant 

[libvirt PATCH 05/14] tests: introduce virgdbusmock to mock GLib DBus functions

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/meson.build|  1 +
 tests/virgdbusmock.c | 85 
 2 files changed, 86 insertions(+)
 create mode 100644 tests/virgdbusmock.c

diff --git a/tests/meson.build b/tests/meson.build
index f0f3d8c1ef..0f3e4bfdd7 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -57,6 +57,7 @@ mock_libs = [
   { 'name': 'vircgroupmock' },
   { 'name': 'virdeterministichashmock' },
   { 'name': 'virfilecachemock' },
+  { 'name': 'virgdbusmock' },
   { 'name': 'virhostcpumock' },
   { 'name': 'virhostdevmock' },
   { 'name': 'virnetdaemonmock' },
diff --git a/tests/virgdbusmock.c b/tests/virgdbusmock.c
new file mode 100644
index 00..7a378a616a
--- /dev/null
+++ b/tests/virgdbusmock.c
@@ -0,0 +1,85 @@
+/*
+ * virgdbusmock.c: mocking of dbus message send/reply
+ *
+ * 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 
+
+#include "virmock.h"
+
+VIR_MOCK_STUB_RET_ARGS(g_bus_get_sync,
+   GDBusConnection *, (GDBusConnection *)0x1,
+   GBusType, type,
+   GCancellable, *cancellable,
+   GError, **error)
+
+VIR_MOCK_STUB_RET_ARGS(g_dbus_address_get_for_bus_sync,
+   gchar *, (gchar *)0x1,
+   GBusType, type,
+   GCancellable *, cancellable,
+   GError **, error)
+
+VIR_MOCK_STUB_RET_ARGS(g_dbus_connection_new_for_address_sync,
+   GDBusConnection *, (GDBusConnection *)0x1,
+   const gchar *, address,
+   GDBusConnectionFlags, flags,
+   GDBusAuthObserver *, observer,
+   GCancellable *, cancellable,
+   GError **, error)
+
+VIR_MOCK_STUB_RET_ARGS(g_dbus_connection_flush_sync,
+   gboolean, true,
+   GDBusConnection *, connection,
+   GCancellable *, cancellable,
+   GError **, error)
+
+VIR_MOCK_STUB_RET_ARGS(g_dbus_connection_close_sync,
+   gboolean, true,
+   GDBusConnection *, connection,
+   GCancellable *, cancellable,
+   GError **, error)
+
+VIR_MOCK_LINK_RET_ARGS(g_dbus_connection_call_sync,
+   GVariant *,
+   GDBusConnection *, connection,
+   const gchar *, bus_name,
+   const gchar *, object_path,
+   const gchar *, interface_name,
+   const gchar *, method_name,
+   GVariant *, parameters,
+   const GVariantType *, reply_type,
+   GDBusCallFlags, flags,
+   gint, timeout_msec,
+   GCancellable *, cancellable,
+   GError **, error)
+
+VIR_MOCK_LINK_RET_ARGS(g_dbus_connection_call_with_unix_fd_list_sync,
+   GVariant *,
+   GDBusConnection *, connection,
+   const gchar *, bus_name,
+   const gchar *, object_path,
+   const gchar *, interface_name,
+   const gchar *, method_name,
+   GVariant *, parameters,
+   const GVariantType *, reply_type,
+   GDBusCallFlags, flags,
+   gint, timeout_msec,
+   GUnixFDList *, fd_list,
+   GUnixFDList **, out_fd_list,
+   GCancellable *, cancellable,
+   GError **, error)
-- 
2.26.2

[libvirt PATCH 07/14] src/util/virfirewalld: convert to use GLib DBus

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/util/virfirewalld.c | 197 
 tests/meson.build   |   4 +-
 tests/networkxml2firewalltest.c |  39 ---
 tests/virfirewalltest.c | 154 ++---
 4 files changed, 180 insertions(+), 214 deletions(-)

diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index c14a6b6e65..69c8b73da0 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -30,7 +30,7 @@
 #include "virerror.h"
 #include "virutil.h"
 #include "virlog.h"
-#include "virdbus.h"
+#include "virgdbus.h"
 #include "virenum.h"
 
 #define VIR_FROM_THIS VIR_FROM_FIREWALLD
@@ -66,7 +66,7 @@ VIR_ENUM_IMPL(virFirewallDBackend,
 int
 virFirewallDIsRegistered(void)
 {
-return virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE);
+return virGDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE);
 }
 
 /**
@@ -82,42 +82,40 @@ virFirewallDIsRegistered(void)
 int
 virFirewallDGetVersion(unsigned long *version)
 {
-int ret = -1;
-DBusConnection *sysbus = virDBusGetSystemBus();
-DBusMessage *reply = NULL;
-g_autofree char *versionStr = NULL;
+GDBusConnection *sysbus = virGDBusGetSystemBus();
+g_autoptr(GVariant) message = NULL;
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(GVariant) gvar = NULL;
+char *versionStr;
 
 if (!sysbus)
 return -1;
 
-if (virDBusCallMethod(sysbus,
-  ,
-  NULL,
-  VIR_FIREWALL_FIREWALLD_SERVICE,
-  "/org/fedoraproject/FirewallD1",
-  "org.freedesktop.DBus.Properties",
-  "Get",
-  "ss",
-  "org.fedoraproject.FirewallD1",
-  "version") < 0)
-goto cleanup;
+message = g_variant_new("(ss)", "org.fedoraproject.FirewallD1", "version");
 
-if (virDBusMessageDecode(reply, "v", "s", ) < 0)
-goto cleanup;
+if (virGDBusCallMethod(sysbus,
+   ,
+   NULL,
+   VIR_FIREWALL_FIREWALLD_SERVICE,
+   "/org/fedoraproject/FirewallD1",
+   "org.freedesktop.DBus.Properties",
+   "Get",
+   message) < 0)
+return -1;
+
+g_variant_get(reply, "(v)", );
+g_variant_get(gvar, "", );
 
 if (virParseVersionString(versionStr, version, false) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to parse firewalld version '%s'"),
versionStr);
-goto cleanup;
+return -1;
 }
 
 VIR_DEBUG("FirewallD version: %s - %lu", versionStr, *version);
 
-ret = 0;
- cleanup:
-virDBusMessageUnref(reply);
-return ret;
+return 0;
 }
 
 /**
@@ -129,42 +127,46 @@ virFirewallDGetVersion(unsigned long *version)
 int
 virFirewallDGetBackend(void)
 {
-DBusConnection *sysbus = virDBusGetSystemBus();
-DBusMessage *reply = NULL;
-virError error;
-g_autofree char *backendStr = NULL;
+GDBusConnection *sysbus = virGDBusGetSystemBus();
+g_autoptr(GVariant) message = NULL;
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(GVariant) gvar = NULL;
+g_autoptr(virError) error = NULL;
+char *backendStr = NULL;
 int backend = -1;
 
 if (!sysbus)
 return -1;
 
-memset(, 0, sizeof(error));
+if (VIR_ALLOC(error) < 0)
+return -1;
 
-if (virDBusCallMethod(sysbus,
-  ,
-  ,
-  VIR_FIREWALL_FIREWALLD_SERVICE,
-  "/org/fedoraproject/FirewallD1/config",
-  "org.freedesktop.DBus.Properties",
-  "Get",
-  "ss",
-  "org.fedoraproject.FirewallD1.config",
-  "FirewallBackend") < 0)
-goto cleanup;
+message = g_variant_new("(ss)",
+"org.fedoraproject.FirewallD1.config",
+"FirewallBackend");
 
-if (error.level == VIR_ERR_ERROR) {
+if (virGDBusCallMethod(sysbus,
+   ,
+   error,
+   VIR_FIREWALL_FIREWALLD_SERVICE,
+   "/org/fedoraproject/FirewallD1/config",
+   "org.freedesktop.DBus.Properties",
+   "Get",
+   message) < 0)
+return -1;
+
+if (error->level == VIR_ERR_ERROR) {
 /* we don't want to log any error in the case that
  * FirewallBackend isn't implemented in this firewalld, since
  * that just means that it is an old version, and only has an
  * iptables backend.
  */
 VIR_DEBUG("Failed to get FirewallBackend 

[libvirt PATCH 00/14] rewrite DBus to use GLib instead of libdbus

[Pavel Hrdina]

Pavel Hrdina (14):
  remove HAL node device driver
  tests: mock libdbus in networkxml2firewalltest
  util: introduce helpers for GLib DBus implementation
  tests/virmock: extend number of arguments
  tests: introduce virgdbusmock to mock GLib DBus functions
  src/util/virpolkit: convert to use GLib DBus
  src/util/virfirewalld: convert to use GLib DBus
  src/util/virsystemd: convert to use GLib DBus
  src/lxc/lxc_controller: convert to use GLib DBus
  src/network/bridge_driver: convert to use GLib DBus
  src/nwfilter/nwfilter_driver: convert to use GLib DBus
  src/remote/remote_daemon: convert to use GLib DBus
  src/rpc/virnetdaemon: convert to use GLib DBus
  drop libdbus from libvirt

 libvirt.spec.in  |3 -
 meson.build  |   42 +-
 meson_options.txt|2 -
 po/POTFILES.in   |3 +-
 src/libvirt_private.syms |   31 +-
 src/libvirt_probes.d |6 -
 src/lxc/lxc_controller.c |6 +-
 src/lxc/meson.build  |1 -
 src/meson.build  |1 -
 src/network/bridge_driver.c  |   81 +-
 src/network/meson.build  |2 -
 src/node_device/meson.build  |5 -
 src/node_device/node_device_driver.c |   10 +-
 src/node_device/node_device_driver.h |5 -
 src/node_device/node_device_hal.c|  843 
 src/node_device/node_device_hal.h|   22 -
 src/nwfilter/meson.build |1 -
 src/nwfilter/nwfilter_driver.c   |  127 +-
 src/remote/remote_daemon.c   |   73 +-
 src/remote/remote_daemon_dispatch.c  |1 -
 src/rpc/meson.build  |1 -
 src/rpc/virnetdaemon.c   |  115 +-
 src/util/meson.build |3 +-
 src/util/virdbus.c   | 1871 --
 src/util/virdbus.h   |   76 --
 src/util/virdbuspriv.h   |   56 -
 src/util/virfirewalld.c  |  197 ++-
 src/util/virgdbus.c  |  425 ++
 src/util/virgdbus.h  |   79 ++
 src/util/virpolkit.c |  115 +-
 src/util/virsystemd.c|  371 ++---
 tests/meson.build|   29 +-
 tests/networkxml2firewalltest.c  |   30 +-
 tests/virdbusmock.c  |   62 -
 tests/virdbustest.c  |  728 --
 tests/virfirewalltest.c  |  154 +--
 tests/virgdbusmock.c |   85 ++
 tests/virmock.h  |   10 +-
 tests/virpolkittest.c|  112 +-
 tests/virsystemdtest.c   |  173 +--
 40 files changed, 1330 insertions(+), 4627 deletions(-)
 delete mode 100644 src/node_device/node_device_hal.c
 delete mode 100644 src/node_device/node_device_hal.h
 delete mode 100644 src/util/virdbus.c
 delete mode 100644 src/util/virdbus.h
 delete mode 100644 src/util/virdbuspriv.h
 create mode 100644 src/util/virgdbus.c
 create mode 100644 src/util/virgdbus.h
 delete mode 100644 tests/virdbusmock.c
 delete mode 100644 tests/virdbustest.c
 create mode 100644 tests/virgdbusmock.c

-- 
2.26.2