Re: [PATCH] qemu: fix code format problem

[yubihong]

OK, Michal. I will do this job later, thanks for your advice.

> On 4/26/20 1:56 PM, yubihong wrote:
> > There are some code format problems in src/libvirt-domain.c. This
> > patch fixes these problems.
> >
> >>From 55cd85345b2dc50f44c1e382563482d40142382b Mon Sep 17 00:00:00
> > 2001
> > From: yubihong 
> > Date: Fri, 24 Apr 2020 17:44:43 +0800
> > Subject: [PATCH] qemu: fix code format problem
> >
> > Signed-off-by:Yu Bihong 
> > ---
> >   src/libvirt-domain.c | 16 
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index
> > a12809c..d659f1b
> > 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -8194,11 +8194,11 @@ virDomainAttachDevice(virDomainPtr domain,
> > const char *xml)
> >   virCheckReadOnlyGoto(conn->flags, error);
> >
> >   if (conn->driver->domainAttachDevice) {
> > -   int ret;
> > -   ret = conn->driver->domainAttachDevice(domain, xml);
> > -   if (ret < 0)
> > -  goto error;
> > -   return ret;
> > +int ret;
> > +ret = conn->driver->domainAttachDevice(domain, xml);
> > +if (ret < 0)
> > +goto error;
> > +return ret;
> >   }
> >
> >   virReportUnsupportedError();
> > @@ -8299,9 +8299,9 @@ virDomainDetachDevice(virDomainPtr domain,
> const
> > char *xml)
> >   if (conn->driver->domainDetachDevice) {
> >   int ret;
> >   ret = conn->driver->domainDetachDevice(domain, xml);
> > - if (ret < 0)
> > - goto error;
> > - return ret;
> > +if (ret < 0)
> > +goto error;
> > +return ret;
> >}
> >
> >   virReportUnsupportedError();
> > --
> > 1.8.3.1
> >
> 
> This patch doesn't apply cleanly. Can you please resend using git-send-email
> as our documentation suggest?
> 
> https://libvirt.org/submitting-patches.html
> 
> The idea looks good, but I guess there are more places in the file that are
> misaligned. Might be worth extending your patch and include them.
> 
> Michal

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

On Wed, Apr 29, 2020 at 10:13:01PM +0800, Eric Blake wrote:
> [meta-comment]
> 
> On 4/29/20 4:35 AM, Yan Zhao wrote:
> > On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote:
> [...]
> > This patchset introduces a migration_version attribute 
> > under sysfs
> >>> of VFIO
> > Mediated devices.
> 
> Hmm, several pages with up to 16 levels of quoting, with editors making 
> the lines ragged, all before I get to the real meat of the email. 
> Remember, it's okay to trim content,...
> 
> >> So why don't we split the difference; lets say that it should start with
> >> the hex PCI Vendor ID.
> >>
> > The problem is for mdev devices, if the parent devices are not PCI devices,
> > they don't have PCI vendor IDs.
> 
> ...to just what you are replying to.
>
sorry for that. next time I'll try to make a better balance between
keeping conversation background and leaving the real meat of the email.

Thanks for reminding.
Yan

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote:

> > > > > > > > > > > > > An mdev type is meant to define a software compatible 
> > > > > > > > > > > > > interface, so in
> > > > > > > > > > > > > the case of mdev->mdev migration, doesn't migrating 
> > > > > > > > > > > > > to a different type
> > > > > > > > > > > > > fail the most basic of compatibility tests that we 
> > > > > > > > > > > > > expect userspace to
> > > > > > > > > > > > > perform?  IOW, if two mdev types are migration 
> > > > > > > > > > > > > compatible, it seems a
> > > > > > > > > > > > > prerequisite to that is that they provide the same 
> > > > > > > > > > > > > software interface,
> > > > > > > > > > > > > which means they should be the same mdev type.
> > > > > > > > > > > > >
> > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how 
> > > > > > > > > > > > > does a
> > > > > > > > > > > > management
> > > > > > > > > > > > > tool begin to even guess what might be compatible?  
> > > > > > > > > > > > > Are we expecting
> > > > > > > > > > > > > libvirt to probe ever device with this attribute in 
> > > > > > > > > > > > > the system?  Is
> > > > > > > > > > > > > there going to be a new class hierarchy created to 
> > > > > > > > > > > > > enumerate all
> > > > > > > > > > > > > possible migrate-able devices?
> > > > > > > > > > > > >
> > > > > > > > > > > > yes, management tool needs to guess and test migration 
> > > > > > > > > > > > compatible
> > > > > > > > > > > > between two devices. But I think it's not the problem 
> > > > > > > > > > > > only for
> > > > > > > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, 
> > > > > > > > > > > > management tool needs
> > > > > > > > > > > > to
> > > > > > > > > > > > first assume that the two mdevs have the same type of 
> > > > > > > > > > > > parent devices
> > > > > > > > > > > > (e.g.their pciids are equal). otherwise, it's still 
> > > > > > > > > > > > enumerating
> > > > > > > > > > > > possibilities.
> > > > > > > > > > > > 
> > > > > > > > > > > > on the other hand, for two mdevs,
> > > > > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1;
> > > > > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2;
> > > > > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow 
> > > > > > > > > > > > migration between
> > > > > > > > > > > > mdev1 <-> mdev2.
> > > > > > > > > > > 
> > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is 
> > > > > > > > > > > equivalent 
> > > > > > > > > > > to 1/4 of pdev2? If we really want to allow such thing 
> > > > > > > > > > > happen, the best
> > > > > > > > > > > choice is to report the same mdev type on both pdev1 and 
> > > > > > > > > > > pdev2.
> > > > > > > > > > I think that's exactly the value of this migration_version 
> > > > > > > > > > interface.
> > > > > > > > > > the management tool can take advantage of this interface to 
> > > > > > > > > > know if two
> > > > > > > > > > devices are migration compatible, no matter they are mdevs, 
> > > > > > > > > > non-mdevs,
> > > > > > > > > > or mix.
> > > > > > > > > > 
> > > > > > > > > > as I know, (please correct me if not right), current 
> > > > > > > > > > libvirt still
> > > > > > > > > > requires manually generating mdev devices, and it just 
> > > > > > > > > > duplicates src vm
> > > > > > > > > > configuration to the target vm.
> > > > > > > > > > for libvirt, currently it's always phys->phys and 
> > > > > > > > > > mdev->mdev (and of the
> > > > > > > > > > same mdev type).
> > > > > > > > > > But it does not justify that hybrid cases should not be 
> > > > > > > > > > allowed. otherwise,
> > > > > > > > > > why do we need to introduce this migration_version 
> > > > > > > > > > interface and leave
> > > > > > > > > > the judgement of migration compatibility to vendor driver? 
> > > > > > > > > > why not simply
> > > > > > > > > > set the criteria to something like "pciids of parent 
> > > > > > > > > > devices are equal,
> > > > > > > > > > and mdev types are equal" ?
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as 
> > > > > > > > > > > Alex pointed out. 
> > > > > > > > > > could you help me understand why it will bring trouble to 
> > > > > > > > > > upper stack?
> > > > > > > > > > 
> > > > > > > > > > I think it just needs to read src migration_version under 
> > > > > > > > > > src dev node,
> > > > > > > > > > and test it in target migration version under target dev 
> > > > > > > > > > node. 
> > > > > > > > > > 
> > > > > > > > > > after all, through this interface we just help the upper 
> > > > > > > > > > layer
> > > > > > > > > > knowing available options through reading and testing, and 
> > > > > > > > > > they decide
> > > > > > > > > > to use it or not.
> > > > > > > > > > 
> > > > > > > > > > > Can we simplify the requirement by allowing only 
> > > > > > > > > > > mdev

Re: [libvirt-ci PATCH] guests: introduce libvirt-dist and libvirt-minimal projects

[Andrea Bolognani]

On Wed, 2020-04-29 at 11:38 +0100, Daniel P. Berrangé wrote:
> Thus this introduces a new project "libvirt-minimal" which lists the
> bare minimum dependencies required to build libvirt.

I don't like the name libvirt-minimal because it appears to follows
the convention used for actual libvirt-based projects such as
libvirt-glib.

What about libvirt+minimal, following the project+variant convention
used for all MinGW builds[1] and in the past for libvirt+website?

> Some projects also wish to have the ability to build against the distro
> provided libvirt instead of the latest git master, and for this purpose
> a "libvirt-dist" project is defined which pulls in the package needed
> for building against the distro libvirt.

I don't like this name either :)

One reason is the same detailed above, but there's another one which
is tied to semantics: in lcitool, "$project" has always been used to
mean "the list of packages necessary to build $project from source".

This is the case for all the projects that exist today[2] and even
for libvirt+minimal, but this one is different: in this case,
libvirt-dist means "the list of packages necessary to build a project
that *depends* on libvirt from source". That's quite different.

I think a better approach would be to once again use variants: for
example, if you wanted to build libvirt-python against the version of
libvirt included in the OS, instead of having something like

  # libvirt-python.yml
  ---
  packages:
- python3
...

  # libvirt-dist.yml
  ---
  packages:
- libvirt

which is then used like

  $ lcitool dockerfile $OS libvirt-dist,libvirt-python

you'd have

  # libvirt-python+dist.yml
  ---
  packages:
- libvirt
- python-3
...

which is used like

  $ lcitool dockerfile $OS libvirt-python+dist

This would achieve the same result with less typing and without
subverting the existing semantics.

Another nice property of this approach is that it naturally extends
to cover projects that depend on more than just libvirt: for example,
we could have

  # libvirt-dbus+dist.yml
  ---
  packages:
- libvirt
- libvirt-glib
...

for use in libvirt-dbus CI.

What do you think?

[...]
> +++ b/guests/vars/mappings.yml
> +  libvirt-devel:
> +rpm: libvirt-devel
> +deb: libvirt-dev
> +pkg: libvirt

The mapping should be called just "libvirt".


[1] Applying this convention to MinGW builds is increasingly
inaccurate now that we have proper multi-arch support, and in
fact at least at the Dockerfile generator level we've gotten
away with this convention; eradicating it completely is something
that I hope I'll be able to spend time on in the near future.
[2] Except for base, blacklist and jenkins, but those are internal
projects which are never exposed to the user so they don't need
to follow the rules as strictly :)
-- 
Andrea Bolognani / Red Hat / Virtualization

[PATCH 09/15] testQemuHotplugCpuPrepare: Allow deprecated commands for non-modern cpu hotplug test

[Peter Krempa]

We have a few cases validating that the code behaves correctly in
pre-modern hotplug era. This is controled by the 'modern' flag for the
test. Since 'cpu-add' command is now deprecated in qemu, there is a
modern replacement for it, and the test output is checked against
expected commands we can skip schema validation for the legacy command.

Signed-off-by: Peter Krempa 
---
 tests/qemuhotplugtest.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 849e7e7636..724f640aef 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -457,6 +457,9 @@ testQemuHotplugCpuPrepare(const char *test,
 if (fail)
 qemuMonitorTestAllowUnusedCommands(data->mon);

+if (!data->modern)
+qemuMonitorTestSkipDeprecatedValidation(data->mon, true);
+
 priv->mon = qemuMonitorTestGetMonitor(data->mon);
 virObjectUnlock(priv->mon);

-- 
2.26.2

[PATCH 11/15] testQemuMonitorJSONqemuMonitorJSONQueryCPUs: Split off test for query-cpus-fast

[Peter Krempa]

Separate the test for the newer command into a new function.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitorjsontest.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 33bad45b96..7e0ab4609c 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1435,10 +1435,6 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void 
*opaque)
 {2, 17626, (char *) "/machine/unattached/device[2]", true},
 {3, 17628, NULL, true},
 };
-struct qemuMonitorQueryCpusEntry expect_fast[] = {
-{0, 17629, (char *) "/machine/unattached/device[0]", false},
-{1, 17630, (char *) "/machine/unattached/device[1]", false},
-};
 g_autoptr(qemuMonitorTest) test = NULL;

 if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
@@ -1483,6 +1479,29 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void 
*opaque)
"}") < 0)
 return -1;

+/* query-cpus */
+if (testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(test, expect_slow,
+  false, 4))
+return -1;
+
+return 0;
+}
+
+
+static int
+testQemuMonitorJSONqemuMonitorJSONQueryCPUsFast(const void *opaque)
+{
+const testGenericData *data = opaque;
+virDomainXMLOptionPtr xmlopt = data->xmlopt;
+struct qemuMonitorQueryCpusEntry expect_fast[] = {
+{0, 17629, (char *) "/machine/unattached/device[0]", false},
+{1, 17630, (char *) "/machine/unattached/device[1]", false},
+};
+g_autoptr(qemuMonitorTest) test = NULL;
+
+if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
+return -1;
+
 if (qemuMonitorTestAddItem(test, "query-cpus-fast",
"{"
"\"return\": ["
@@ -1501,11 +1520,6 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void 
*opaque)
"}") < 0)
 return -1;

-/* query-cpus */
-if (testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(test, expect_slow,
-  false, 4))
-return -1;
-
 /* query-cpus-fast */
 if (testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(test, expect_fast,
   true, 2))
@@ -3236,6 +3250,7 @@ mymain(void)
 DO_TEST(qemuMonitorJSONGetTargetArch);
 DO_TEST(qemuMonitorJSONGetMigrationCapabilities);
 DO_TEST(qemuMonitorJSONQueryCPUs);
+DO_TEST(qemuMonitorJSONQueryCPUsFast);
 DO_TEST(qemuMonitorJSONGetVirtType);
 DO_TEST(qemuMonitorJSONSendKey);
 DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability);
-- 
2.26.2

[PATCH 06/15] testutilsqemuschema: Use automatic variable clearing where possible

[Peter Krempa]

Refactor all cleanup to avoid manual clearing, unnecessary labels and
return value variables.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 81 +
 1 file changed, 29 insertions(+), 52 deletions(-)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 60409a0f91..a43cbe2f91 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -123,36 +123,33 @@ testQEMUSchemaValidateObjectMember(const char *key,
void *opaque)
 {
 struct testQEMUSchemaValidateObjectMemberData *data = opaque;
-virJSONValuePtr keymember = NULL;
+g_autoptr(virJSONValue) keymember = NULL;
 const char *keytype;
 virJSONValuePtr keyschema = NULL;
-int ret = -1;
+int rc;

 virBufferStrcat(data->debug, key, ": ", NULL);

 /* lookup 'member' entry for key */
 if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, 
data->rootmembers))) {
-virBufferAddLit(data->debug, "ERROR: attribute not in schema");
-goto cleanup;
+virBufferAddLit(data->debug, "ERROR: attribute not in schema\n");
+return -1;
 }

 /* lookup schema entry for keytype */
 if (!(keytype = virJSONValueObjectGetString(keymember, "type")) ||
 !(keyschema = virHashLookup(data->schema, keytype))) {
-virBufferAsprintf(data->debug, "ERROR: can't find schema for type 
'%s'",
+virBufferAsprintf(data->debug, "ERROR: can't find schema for type 
'%s'\n",
   NULLSTR(keytype));
-ret = -2;
-goto cleanup;
+return -2;
 }

 /* recurse */
-ret = testQEMUSchemaValidateRecurse(value, keyschema, data->schema,
+rc = testQEMUSchemaValidateRecurse(value, keyschema, data->schema,
 data->debug);

- cleanup:
 virBufferAddLit(data->debug, "\n");
-virJSONValueFree(keymember);
-return ret;
+return rc;
 }


@@ -188,13 +185,12 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
  virBufferPtr debug)
 {
 size_t i;
-virJSONValuePtr variants = NULL;
+g_autoptr(virJSONValue) variants = NULL;
 virJSONValuePtr variant;
 virJSONValuePtr variantschema;
 virJSONValuePtr variantschemamembers;
 virJSONValuePtr rootmembers;
 const char *varianttype = NULL;
-int ret = -1;

 if (!(variants = virJSONValueObjectStealArray(root, "variants"))) {
 virBufferAddLit(debug, "ERROR: missing 'variants' in schema\n");
@@ -214,8 +210,7 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
 if (!varianttype) {
 virBufferAsprintf(debug, "ERROR: variant '%s' for discriminator '%s' 
not found\n",
   variantname, variantfield);
-goto cleanup;
-
+return -1;
 }

 if (!(variantschema = virHashLookup(schema, varianttype)) ||
@@ -223,8 +218,7 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
 virBufferAsprintf(debug,
   "ERROR: missing schema or schema members for variant 
'%s'(%s)\n",
   variantname, varianttype);
-ret = -2;
-goto cleanup;
+return -2;
 }

 rootmembers = virJSONValueObjectGetArray(root, "members");
@@ -232,15 +226,10 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
 if (virJSONValueArrayForeachSteal(variantschemamembers,
   
testQEMUSchemaValidateObjectMergeVariantMember,
   rootmembers) < 0) {
-ret = -2;
-goto cleanup;
+return -2;
 }

-ret = 0;
-
- cleanup:
-virJSONValueFree(variants);
-return ret;
+return 0;
 }


@@ -269,10 +258,9 @@ testQEMUSchemaValidateObject(virJSONValuePtr obj,
 {
 struct testQEMUSchemaValidateObjectMemberData data = { NULL, schema,
debug, false };
-virJSONValuePtr localroot = NULL;
+g_autoptr(virJSONValue) localroot = NULL;
 const char *variantfield;
 const char *variantname;
-int ret = -1;

 if (virJSONValueGetType(obj) != VIR_JSON_TYPE_OBJECT) {
 virBufferAddLit(debug, "ERROR: not an object");
@@ -283,23 +271,21 @@ testQEMUSchemaValidateObject(virJSONValuePtr obj,
 virBufferAdjustIndent(debug, 3);

 /* copy schema */
-if (!(localroot = virJSONValueCopy(root))) {
-ret = -2;
-goto cleanup;
-}
+if (!(localroot = virJSONValueCopy(root)))
+return -2;

 /* remove variant */
 if ((variantfield = virJSONValueObjectGetString(localroot, "tag"))) {
 if (!(variantname = virJSONValueObjectGetString(obj, variantfield))) {
 virBufferAsprintf(debug, "ERROR: missing variant discriminator 
attribute '%s'\n",
   variantfield);
-goto cleanup;
+

[PATCH 07/15] testutilsqemuschema: Pass in 'schema' and 'debug' variables to workers in a struct

[Peter Krempa]

Refactor the code so that we pass in the repeated 'schema' and 'debug'
arguments via a new struct testQEMUSchemaValidateCtxt. This will
simplify adding new parameters in the future.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 152 ++--
 1 file changed, 76 insertions(+), 76 deletions(-)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index a43cbe2f91..b449171d15 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -21,16 +21,21 @@
 #include "testutilsqemuschema.h"
 #include "qemu/qemu_qapi.h"

+struct testQEMUSchemaValidateCtxt {
+virHashTablePtr schema;
+virBufferPtr debug;
+};
+
+
 static int
 testQEMUSchemaValidateRecurse(virJSONValuePtr obj,
   virJSONValuePtr root,
-  virHashTablePtr schema,
-  virBufferPtr debug);
+  struct testQEMUSchemaValidateCtxt *ctxt);

 static int
 testQEMUSchemaValidateBuiltin(virJSONValuePtr obj,
   virJSONValuePtr root,
-  virBufferPtr debug)
+  struct testQEMUSchemaValidateCtxt *ctxt)
 {
 const char *t = virJSONValueObjectGetString(root, "json-type");
 const char *s = NULL;
@@ -81,17 +86,16 @@ testQEMUSchemaValidateBuiltin(virJSONValuePtr obj,

  cleanup:
 if (ret == 0)
-virBufferAsprintf(debug, "'%s': OK", s);
+virBufferAsprintf(ctxt->debug, "'%s': OK", s);
 else
-virBufferAsprintf(debug, "ERROR: expected type '%s', actual type %d",
+virBufferAsprintf(ctxt->debug, "ERROR: expected type '%s', actual type 
%d",
   t, virJSONValueGetType(obj));
 return ret;
 }

 struct testQEMUSchemaValidateObjectMemberData {
 virJSONValuePtr rootmembers;
-virHashTablePtr schema;
-virBufferPtr debug;
+struct testQEMUSchemaValidateCtxt *ctxt;
 bool missingMandatory;
 };

@@ -128,27 +132,26 @@ testQEMUSchemaValidateObjectMember(const char *key,
 virJSONValuePtr keyschema = NULL;
 int rc;

-virBufferStrcat(data->debug, key, ": ", NULL);
+virBufferStrcat(data->ctxt->debug, key, ": ", NULL);

 /* lookup 'member' entry for key */
 if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, 
data->rootmembers))) {
-virBufferAddLit(data->debug, "ERROR: attribute not in schema\n");
+virBufferAddLit(data->ctxt->debug, "ERROR: attribute not in schema\n");
 return -1;
 }

 /* lookup schema entry for keytype */
 if (!(keytype = virJSONValueObjectGetString(keymember, "type")) ||
-!(keyschema = virHashLookup(data->schema, keytype))) {
-virBufferAsprintf(data->debug, "ERROR: can't find schema for type 
'%s'\n",
+!(keyschema = virHashLookup(data->ctxt->schema, keytype))) {
+virBufferAsprintf(data->ctxt->debug, "ERROR: can't find schema for 
type '%s'\n",
   NULLSTR(keytype));
 return -2;
 }

 /* recurse */
-rc = testQEMUSchemaValidateRecurse(value, keyschema, data->schema,
-data->debug);
+rc = testQEMUSchemaValidateRecurse(value, keyschema, data->ctxt);

-virBufferAddLit(data->debug, "\n");
+virBufferAddLit(data->ctxt->debug, "\n");
 return rc;
 }

@@ -181,8 +184,7 @@ static int
 testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr root,
  const char *variantfield,
  const char *variantname,
- virHashTablePtr schema,
- virBufferPtr debug)
+ struct testQEMUSchemaValidateCtxt 
*ctxt)
 {
 size_t i;
 g_autoptr(virJSONValue) variants = NULL;
@@ -193,7 +195,7 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
 const char *varianttype = NULL;

 if (!(variants = virJSONValueObjectStealArray(root, "variants"))) {
-virBufferAddLit(debug, "ERROR: missing 'variants' in schema\n");
+virBufferAddLit(ctxt->debug, "ERROR: missing 'variants' in schema\n");
 return -2;
 }

@@ -208,14 +210,14 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr 
root,
 }

 if (!varianttype) {
-virBufferAsprintf(debug, "ERROR: variant '%s' for discriminator '%s' 
not found\n",
+virBufferAsprintf(ctxt->debug, "ERROR: variant '%s' for discriminator 
'%s' not found\n",
   variantname, variantfield);
 return -1;
 }

-if (!(variantschema = virHashLookup(schema, varianttype)) ||
+if (!(variantschema = virHashLookup(ctxt->schema, varianttype)) ||
 !(variantschemamembers = virJSONValueObjectGetArray(variantschema, 
"members"))) {
-virBufferAsprintf(debug,
+virBufferAsprintf(ctxt->debug,
   "

[PATCH 08/15] testQEMUSchemaValidate(Command): Allow skipping validation of deprecated fields

[Peter Krempa]

Some test cases are used to validate interactions with old qemu. We need
to skip validation of deprecation with those once it will be added.

In case of commands which were already replaced by code based on
capabilities we can skip the full validation once the command is
removed.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c| 14 +++---
 tests/qemumonitorjsontest.c  |  3 ++-
 tests/qemumonitortestutils.c |  5 -
 tests/testutilsqemuschema.c  | 17 +++--
 tests/testutilsqemuschema.h  |  3 +++
 5 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 8001807552..604e71bba7 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -86,7 +86,7 @@ testBackingXMLjsonXML(const void *args)

 if (!data->legacy) {
 if (testQEMUSchemaValidate(backendprops, data->schemaroot,
-   data->schema, &debug) < 0) {
+   data->schema, false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);
 g_autofree char *debugprops = virJSONValueToString(backendprops, 
true);

@@ -168,7 +168,7 @@ testJSONtoJSON(const void *args)
 return -1;

 if (testQEMUSchemaValidate(jsonsrcout, data->schemaroot,
-   data->schema, &debug) < 0) {
+   data->schema, false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);

 VIR_TEST_VERBOSE("json does not conform to QAPI schema");
@@ -342,7 +342,7 @@ testQemuDiskXMLToPropsValidateSchema(const void *opaque)
 g_auto(virBuffer) debug = VIR_BUFFER_INITIALIZER;

 if (testQEMUSchemaValidate(data->images[i].formatprops, 
data->schemaroot,
-   data->schema, &debug) < 0) {
+   data->schema, false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);
 g_autofree char *propsstr = 
virJSONValueToString(data->images[i].formatprops, true);
 VIR_TEST_VERBOSE("json does not conform to QAPI schema");
@@ -354,7 +354,7 @@ testQemuDiskXMLToPropsValidateSchema(const void *opaque)
 virBufferFreeAndReset(&debug);

 if (testQEMUSchemaValidate(data->images[i].storageprops, 
data->schemaroot,
-   data->schema, &debug) < 0) {
+   data->schema, false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);
 g_autofree char *propsstr = 
virJSONValueToString(data->images[i].storageprops, true);
 VIR_TEST_VERBOSE("json does not conform to QAPI schema");
@@ -366,7 +366,7 @@ testQemuDiskXMLToPropsValidateSchema(const void *opaque)
 virBufferFreeAndReset(&debug);

 if (testQEMUSchemaValidate(data->images[i].storagepropssrc, 
data->schemaroot,
-   data->schema, &debug) < 0) {
+   data->schema, false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);
 g_autofree char *propsstr = 
virJSONValueToString(data->images[i].storagepropssrc, true);
 VIR_TEST_VERBOSE("json does not conform to QAPI schema");
@@ -544,7 +544,7 @@ testQemuImageCreate(const void *opaque)
 return -1;

 if (testQEMUSchemaValidate(formatprops, data->schemaroot, data->schema,
-   &debug) < 0) {
+   false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);
 VIR_TEST_VERBOSE("blockdev-create format json does not conform to 
QAPI schema");
 VIR_TEST_DEBUG("json:\n%s\ndoes not match schema. Debug output:\n 
%s",
@@ -559,7 +559,7 @@ testQemuImageCreate(const void *opaque)
 return -1;

 if (testQEMUSchemaValidate(protocolprops, data->schemaroot, 
data->schema,
-   &debug) < 0) {
+   false, &debug) < 0) {
 g_autofree char *debugmsg = virBufferContentAndReset(&debug);
 VIR_TEST_VERBOSE("blockdev-create protocol json does not conform 
to QAPI schema");
 VIR_TEST_DEBUG("json:\n%s\ndoes not match schema. Debug output:\n 
%s",
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 60c816d1d1..fce88083b9 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2842,7 +2842,8 @@ testQAPISchemaValidate(const void *opaque)
 if (!(json = virJSONValueFromString(data->json)))
 goto cleanup;

-if ((testQEMUSchemaValidate(json, schemaroot, data->schema, &debug) == 0) 
!= data->success) {
+if ((testQEMUSchemaValidate(json, schemaroot, data->schema, false,
+&debug) == 0) != data->success) 

[PATCH 00/15] tests: qemu: Detect deprecation in the QMP schema (deprecation part 1)

[Peter Krempa]

Recent qemu versions started adding the 'deprecated' feature to commands
and fields. Use this data to validate that our code doesn't rely on
deprecated commands. It turns out that we still use some old migration
commands (See 14/15) and add validator to catch anything deprecated
early.

Peter Krempa (15):
  testQemuHotplugCpuPrepare: Allow unused monitor commands only on
failure
  testutilsqemuschema: Introduce testQEMUSchemaValidateCommand
  qemuMonitorTestProcessCommandDefaultValidate: Use
testQEMUSchemaValidateCommand
  qemuMonitorTestProcessCommandDefaultValidate: Clean up return value
use
  qemumonitortestutils: Introduce
qemuMonitorTestSkipDeprecatedValidation
  testutilsqemuschema: Use automatic variable clearing where possible
  testutilsqemuschema: Pass in 'schema' and 'debug' variables to workers
in a struct
  testQEMUSchemaValidate(Command): Allow skipping validation of
deprecated fields
  testQemuHotplugCpuPrepare: Allow deprecated commands for non-modern
cpu hotplug test
  qemumonitorjsontest: Add infrastructure for generated tests of
deprecated commands
  testQemuMonitorJSONqemuMonitorJSONQueryCPUs: Split off test for
query-cpus-fast
  qemumonitorjsontest: Allow use of deprecated 'query-cpus'
  qemumonitorjsontest: Allow use of deprecated 'cpu-add' and 'change'
command
  qemumonitorjsontest: Mark recently deprecated migration command in our
tests
  testQEMUSchemaValidate*: Reject usage of fields with 'deprecated' set

 tests/qemublocktest.c|  14 +-
 tests/qemuhotplugtest.c  |  11 +-
 tests/qemumonitorjsontest.c  |  67 +--
 tests/qemumonitortestutils.c |  66 ---
 tests/qemumonitortestutils.h |   2 +
 tests/testutilsqemuschema.c  | 334 ++-
 tests/testutilsqemuschema.h  |   9 +
 7 files changed, 325 insertions(+), 178 deletions(-)

-- 
2.26.2

[PATCH 13/15] qemumonitorjsontest: Allow use of deprecated 'cpu-add' and 'change' command

[Peter Krempa]

Modify the generated test cases for the 'cpu-add' and 'change' command
which are deprecated by qemu. We now use device-add and
blockdev-change-media instead so we are okay if they will be removed.

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

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index eaaabe9a47..82a8122f29 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -3214,9 +3214,9 @@ mymain(void)
 DO_TEST_GEN(qemuMonitorJSONSetPassword);
 DO_TEST_GEN(qemuMonitorJSONExpirePassword);
 DO_TEST_GEN(qemuMonitorJSONSetBalloon);
-DO_TEST_GEN(qemuMonitorJSONSetCPU);
+DO_TEST_GEN_DEPRECATED(qemuMonitorJSONSetCPU, true);
 DO_TEST_GEN(qemuMonitorJSONEjectMedia);
-DO_TEST_GEN(qemuMonitorJSONChangeMedia);
+DO_TEST_GEN_DEPRECATED(qemuMonitorJSONChangeMedia, true);
 DO_TEST_GEN(qemuMonitorJSONSaveVirtualMemory);
 DO_TEST_GEN(qemuMonitorJSONSavePhysicalMemory);
 DO_TEST_GEN(qemuMonitorJSONSetMigrationSpeed);
-- 
2.26.2

[PATCH 04/15] qemuMonitorTestProcessCommandDefaultValidate: Clean up return value use

[Peter Krempa]

We no longer return the error via the monitor, so the function no longer
returns '1'. Remove the mention from comment and fix callers to stop
looking for the return value of '1'.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitortestutils.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index df780643dd..3dc4b674f3 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -529,8 +529,7 @@ qemuMonitorTestHandlerDataFree(void *opaque)
 }


-/* Returns -1 on error, 0 if validation was successful/not necessary, 1 if
- * the validation has failed, and the reply was properly constructed */
+/* Returns -1 on error, 0 if validation was successful/not necessary */
 static int
 qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test,
  const char *cmdname,
@@ -585,7 +584,6 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr 
test,
 g_autoptr(virJSONValue) val = NULL;
 virJSONValuePtr cmdargs = NULL;
 const char *cmdname;
-int rc;

 if (!(val = virJSONValueFromString(cmdstr)))
 return -1;
@@ -596,10 +594,8 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr 
test,
 }

 cmdargs = virJSONValueObjectGet(val, "arguments");
-if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, 
cmdargs)) < 0)
+if (qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, cmdargs) < 
0)
 return -1;
-if (rc == 1)
-return 0;

 if (data->command_name && STRNEQ(data->command_name, cmdname)) {
 qemuMonitorTestErrorInvalidCommand(data->command_name, cmdname);
@@ -641,7 +637,6 @@ qemuMonitorTestProcessCommandVerbatim(qemuMonitorTestPtr 
test,
 virJSONValuePtr cmdargs;
 const char *cmdname;
 int ret = -1;
-int rc;

 /* JSON strings will be reformatted to simplify checking */
 if (!(json = virJSONValueFromString(cmdstr)) ||
@@ -653,12 +648,8 @@ qemuMonitorTestProcessCommandVerbatim(qemuMonitorTestPtr 
test,
 /* in this case we do a best-effort schema check if we can find the 
command */
 if ((cmdname = virJSONValueObjectGetString(json, "execute"))) {
 cmdargs = virJSONValueObjectGet(json, "arguments");
-
-if ((rc = qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, 
cmdargs)) < 0)
+if (qemuMonitorTestProcessCommandDefaultValidate(test, cmdname, 
cmdargs) < 0)
 return -1;
-
-if (rc == 1)
-return 0;
 }

 if (STREQ(data->command_name, cmdstr)) {
-- 
2.26.2

[PATCH 01/15] testQemuHotplugCpuPrepare: Allow unused monitor commands only on failure

[Peter Krempa]

Only tests expected to fail should allow unused commads as the normal
tests will consume all of them.

Signed-off-by: Peter Krempa 
---
 tests/qemuhotplugtest.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 9a215ab303..849e7e7636 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -409,6 +409,7 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData 
*data)
 static struct testQemuHotplugCpuData *
 testQemuHotplugCpuPrepare(const char *test,
   bool modern,
+  bool fail,
   virHashTablePtr qmpschema)
 {
 qemuDomainObjPrivatePtr priv = NULL;
@@ -453,7 +454,8 @@ testQemuHotplugCpuPrepare(const char *test,
  &driver, data->vm, 
qmpschema)))
 goto error;

-qemuMonitorTestAllowUnusedCommands(data->mon);
+if (fail)
+qemuMonitorTestAllowUnusedCommands(data->mon);

 priv->mon = qemuMonitorTestGetMonitor(data->mon);
 virObjectUnlock(priv->mon);
@@ -528,7 +530,7 @@ testQemuHotplugCpuGroup(const void *opaque)
 int rc;

 if (!(data = testQemuHotplugCpuPrepare(params->test, params->modern,
-   params->schema)))
+   params->fail, params->schema)))
 return -1;

 rc = qemuDomainSetVcpusInternal(&driver, data->vm, data->vm->def,
@@ -565,7 +567,7 @@ testQemuHotplugCpuIndividual(const void *opaque)
 int rc;

 if (!(data = testQemuHotplugCpuPrepare(params->test, params->modern,
-   params->schema)))
+   params->fail, params->schema)))
 return -1;

 if (virBitmapParse(params->cpumap, &map, 128) < 0)
-- 
2.26.2

[PATCH 05/15] qemumonitortestutils: Introduce qemuMonitorTestSkipDeprecatedValidation

[Peter Krempa]

Upcoming patches will add validation which rejects objects with the
'deprecated' feature in the QMP schema. To support tests which deal with
legacy properties in case when a command or argument is marked as
deprecated or removed by qemu qemuMonitorTestSkipDeprecatedValidation
will allow configuring the tests to ignore such errors.

In case of commands/features which are not yet replaced, the
'allowRemoved' bool should not be set to provide a hard notification
once qemu drops the command.

Note that at this point 'allowRemoved' only includes whole commands, but
not specific properties.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitortestutils.c | 28 
 tests/qemumonitortestutils.h |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 3dc4b674f3..0d9427f1d1 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -59,6 +59,9 @@ struct _qemuMonitorTest {

 bool allowUnusedCommands;

+bool skipValidationDeprecated;
+bool skipValidationRemoved;
+
 char *incoming;
 size_t incomingLength;
 size_t incomingCapacity;
@@ -1298,6 +1301,31 @@ qemuMonitorTestAllowUnusedCommands(qemuMonitorTestPtr 
test)
 }


+/**
+ * qemuMonitorTestSkipDeprecatedValidation:
+ * @test: test monitor object
+ * @allowRemoved: don't produce errors if command was removed from QMP schema
+ *
+ * By default if the QMP schema is provided all test items/commands are
+ * validated against the schema. This function allows to override the 
validation
+ * and additionally if @allowRemoved is true and if such a command is no longer
+ * present in the QMP, only a warning is printed.
+ *
+ * '@allowRemoved' must be used only if a suitable replacement is already in
+ * use and the code tests legacy interactions.
+ *
+ * Note that currently '@allowRemoved' influences only removed commands. If an
+ * argument is removed it will still fail validation.
+ */
+void
+qemuMonitorTestSkipDeprecatedValidation(qemuMonitorTestPtr test,
+bool allowRemoved)
+{
+test->skipValidationDeprecated = true;
+test->skipValidationRemoved = allowRemoved;
+}
+
+
 static int
 qemuMonitorTestFullAddItem(qemuMonitorTestPtr test,
const char *filename,
diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h
index f45e85..1073ef4100 100644
--- a/tests/qemumonitortestutils.h
+++ b/tests/qemumonitortestutils.h
@@ -51,6 +51,8 @@ void 
*qemuMonitorTestItemGetPrivateData(qemuMonitorTestItemPtr item);
 int qemuMonitorTestAddErrorResponse(qemuMonitorTestPtr test, const char 
*errmsg, ...);

 void qemuMonitorTestAllowUnusedCommands(qemuMonitorTestPtr test);
+void qemuMonitorTestSkipDeprecatedValidation(qemuMonitorTestPtr test,
+ bool allowRemoved);

 int qemuMonitorTestAddItem(qemuMonitorTestPtr test,
const char *command_name,
-- 
2.26.2

[PATCH 14/15] qemumonitorjsontest: Mark recently deprecated migration command in our tests

[Peter Krempa]

"migrate_set_downtime", "migrate_set_speed", and
"query-migrate-cache-size" were marked as deprecated in the QMP schema
in qemu 5.0. Since libvirt still actively uses them we must not mark
them as okay to be missing, but still mark them as deprecated, so that
we can add tests for deprecated commands.

The replacement of the command usage in libvirt is tracked by:
https://bugzilla.redhat.com/show_bug.cgi?id=1829543
https://bugzilla.redhat.com/show_bug.cgi?id=1829544
https://bugzilla.redhat.com/show_bug.cgi?id=1829545

Signed-off-by: Peter Krempa 
---
 tests/qemumonitorjsontest.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 82a8122f29..f58b18a110 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1889,6 +1889,8 @@ 
testQemuMonitorJSONqemuMonitorJSONGetMigrationCacheSize(const void *opaque)
 if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
 return -1;

+qemuMonitorTestSkipDeprecatedValidation(test, false);
+
 if (qemuMonitorTestAddItem(test, "query-migrate-cache-size",
"{"
"\"return\": 67108864,"
@@ -3219,8 +3221,8 @@ mymain(void)
 DO_TEST_GEN_DEPRECATED(qemuMonitorJSONChangeMedia, true);
 DO_TEST_GEN(qemuMonitorJSONSaveVirtualMemory);
 DO_TEST_GEN(qemuMonitorJSONSavePhysicalMemory);
-DO_TEST_GEN(qemuMonitorJSONSetMigrationSpeed);
-DO_TEST_GEN(qemuMonitorJSONSetMigrationDowntime);
+DO_TEST_GEN_DEPRECATED(qemuMonitorJSONSetMigrationSpeed, false);
+DO_TEST_GEN_DEPRECATED(qemuMonitorJSONSetMigrationDowntime, false);
 DO_TEST_GEN(qemuMonitorJSONMigrate);
 DO_TEST_GEN(qemuMonitorJSONDump);
 DO_TEST_GEN(qemuMonitorJSONGraphicsRelocate);
-- 
2.26.2

[PATCH 15/15] testQEMUSchemaValidate*: Reject usage of fields with 'deprecated' set

[Peter Krempa]

Make our QMP schema validator reject any use of schema entries which
were deprecated by QEMU except for those whitelisted.

This will allow us to catch this before qemu actually removed what we'd
still use.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 54 -
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index f94a415b18..898be68b0a 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -445,6 +445,47 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
 }


+static int
+testQEMUSchemaValidateDeprecated(virJSONValuePtr root,
+ const char *name,
+ struct testQEMUSchemaValidateCtxt *ctxt)
+{
+virJSONValuePtr features = virJSONValueObjectGetArray(root, "features");
+size_t nfeatures;
+size_t i;
+
+if (!features)
+return 0;
+
+nfeatures = virJSONValueArraySize(features);
+
+for (i = 0; i < nfeatures; i++) {
+virJSONValuePtr cur = virJSONValueArrayGet(features, i);
+const char *curstr;
+
+if (!cur ||
+!(curstr = virJSONValueGetString(cur))) {
+virBufferAsprintf(ctxt->debug, "ERROR: features of '%s' are 
malformed", name);
+return -2;
+}
+
+if (STREQ(curstr, "deprecated")) {
+if (ctxt->allowDeprecated) {
+virBufferAsprintf(ctxt->debug, "WARNING: '%s' is deprecated", 
name);
+if (virTestGetVerbose())
+g_fprintf(stderr, "\nWARNING: '%s' is deprecated\n", name);
+return 0;
+} else {
+virBufferAsprintf(ctxt->debug, "ERROR: '%s' is deprecated", 
name);
+return -1;
+}
+}
+}
+
+return 0;
+}
+
+
 static int
 testQEMUSchemaValidateRecurse(virJSONValuePtr obj,
   virJSONValuePtr root,
@@ -452,6 +493,10 @@ testQEMUSchemaValidateRecurse(virJSONValuePtr obj,
 {
 const char *n = virJSONValueObjectGetString(root, "name");
 const char *t = virJSONValueObjectGetString(root, "meta-type");
+int rc;
+
+if ((rc = testQEMUSchemaValidateDeprecated(root, n, ctxt)) < 0)
+return rc;

 if (STREQ_NULLABLE(t, "builtin")) {
 return testQEMUSchemaValidateBuiltin(obj, root, ctxt);
@@ -525,7 +570,7 @@ testQEMUSchemaValidateCommand(const char *command,
   virJSONValuePtr arguments,
   virHashTablePtr schema,
   bool allowDeprecated,
-  bool allowRemoved G_GNUC_UNUSED,
+  bool allowRemoved,
   virBufferPtr debug)
 {
 struct testQEMUSchemaValidateCtxt ctxt = { .schema = schema,
@@ -535,13 +580,20 @@ testQEMUSchemaValidateCommand(const char *command,
 g_autoptr(virJSONValue) emptyargs = NULL;
 virJSONValuePtr schemarootcommand;
 virJSONValuePtr schemarootarguments;
+int rc;

 if (virQEMUQAPISchemaPathGet(command, schema, &schemarootcommand) < 0 ||
 !schemarootcommand) {
+if (allowRemoved)
+return 0;
+
 virBufferAsprintf(debug, "ERROR: command '%s' not found in the 
schema", command);
 return -1;
 }

+if ((rc = testQEMUSchemaValidateDeprecated(schemarootcommand, command, 
&ctxt)) < 0)
+return rc;
+
 if (!arguments)
 arguments = emptyargs = virJSONValueNewObject();

-- 
2.26.2

[PATCH 03/15] qemuMonitorTestProcessCommandDefaultValidate: Use testQEMUSchemaValidateCommand

[Peter Krempa]

Remove the ad-hoc command validation in favor of the new helper.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitortestutils.c | 20 +---
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 4ca29ab061..df780643dd 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -537,9 +537,6 @@ 
qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test,
  virJSONValuePtr args)
 {
 g_auto(virBuffer) debug = VIR_BUFFER_INITIALIZER;
-virJSONValuePtr schemaroot;
-g_autoptr(virJSONValue) emptyargs = NULL;
-g_autofree char *schemapath = NULL;

 if (!test->qapischema)
 return 0;
@@ -555,20 +552,13 @@ 
qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test,
 if (STREQ(cmdname, "device_add"))
 return 0;

-schemapath = g_strdup_printf("%s/arg-type", cmdname);
-
-if (virQEMUQAPISchemaPathGet(schemapath, test->qapischema, &schemaroot) < 
0 ||
-!schemaroot) {
-qemuMonitorTestError("command '%s' not found in QAPI schema", cmdname);
-return -1;
-}
+if (testQEMUSchemaValidateCommand(cmdname, args, test->qapischema, &debug) 
< 0) {
+if (virTestGetDebug() == 2) {
+g_autofree char *argstr = NULL;

-if (!args)
-args = emptyargs = virJSONValueNewObject();
+if (args)
+argstr = virJSONValueToString(args, true);

-if (testQEMUSchemaValidate(args, schemaroot, test->qapischema, &debug) < 
0) {
-if (virTestGetDebug() == 2) {
-g_autofree char *argstr = virJSONValueToString(args, true);
 fprintf(stderr,
 "\nfailed to validate arguments of '%s' against QAPI 
schema\n"
 "args:\n%s\nvalidator output:\n %s\n",
-- 
2.26.2

[PATCH 10/15] qemumonitorjsontest: Add infrastructure for generated tests of deprecated commands

[Peter Krempa]

For sanity-chcecking of deprecated commands which are still used on some
old code paths which used the simple generated test cases add a
mechanism to mark them as deprecated so schema checking can be skipped.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitorjsontest.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index fce88083b9..33bad45b96 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -46,6 +46,8 @@ struct _testQemuMonitorJSONSimpleFuncData {
 virDomainXMLOptionPtr xmlopt;
 const char *reply;
 virHashTablePtr schema;
+bool allowDeprecated;
+bool allowRemoved;
 };

 typedef struct _testGenericData testGenericData;
@@ -1278,6 +1280,9 @@ testQemuMonitorJSON ## funcName(const void *opaque) \
  \
 if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) \
 return -1; \
+ \
+if (data->allowDeprecated) \
+qemuMonitorTestSkipDeprecatedValidation(test, data->allowRemoved); \
  \
 if (!reply) \
 reply = "{\"return\":{}}"; \
@@ -3121,13 +3126,19 @@ mymain(void)
 if (virTestRun(# FNC, testQemuMonitorJSONSimpleFunc, &simpleFunc) < 0) \
 ret = -1

-#define DO_TEST_GEN(name, ...) \
+#define DO_TEST_GEN_FULL(name, dpr, rmvd, ...) \
 simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.xmlopt = driver.xmlopt, 
\
+  .allowDeprecated = dpr, \
+  .allowRemoved = rmvd, \
   .schema = 
qapiData.schema \
  __VA_ARGS__ }; \
 if (virTestRun(# name, testQemuMonitorJSON ## name, &simpleFunc) < 0) \
 ret = -1

+#define DO_TEST_GEN(name, ...) DO_TEST_GEN_FULL(name, false, false, 
__VA_ARGS__)
+#define DO_TEST_GEN_DEPRECATED(name, removed, ...) \
+DO_TEST_GEN_FULL(name, true, removed, __VA_ARGS__)
+
 #define DO_TEST_CPU_DATA(name) \
 do { \
 struct testCPUData data = { name, driver.xmlopt, qapiData.schema }; \
-- 
2.26.2

[PATCH 02/15] testutilsqemuschema: Introduce testQEMUSchemaValidateCommand

[Peter Krempa]

The new helper splits out all steps necessary to validate a QMP command
against the schema.

Signed-off-by: Peter Krempa 
---
 tests/testutilsqemuschema.c | 44 +
 tests/testutilsqemuschema.h |  6 +
 2 files changed, 50 insertions(+)

diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 7b82ff27b2..60409a0f91 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -517,6 +517,50 @@ testQEMUSchemaValidate(virJSONValuePtr obj,
 }


+/**
+ * testQEMUSchemaValidateCommand:
+ * @command: command to validate
+ * @arguments: arguments of @command to validate
+ * @schema: hash table containing schema entries
+ * @debug: a virBuffer which will be filled with debug information if provided
+ *
+ * Validates whether @command and it's @arguments conforms to the QAPI schema
+ * passed in via @schema. Returns 0, if the command and args matches @schema,
+ * -1 if it does not and -2 if there is a problem with the schema or with
+ *  internals.
+ *
+ * @debug is filled with information regarding the validation process
+ */
+int
+testQEMUSchemaValidateCommand(const char *command,
+  virJSONValuePtr arguments,
+  virHashTablePtr schema,
+  virBufferPtr debug)
+{
+g_autofree char *schemapatharguments = g_strdup_printf("%s/arg-type", 
command);
+g_autoptr(virJSONValue) emptyargs = NULL;
+virJSONValuePtr schemarootcommand;
+virJSONValuePtr schemarootarguments;
+
+if (virQEMUQAPISchemaPathGet(command, schema, &schemarootcommand) < 0 ||
+!schemarootcommand) {
+virBufferAsprintf(debug, "ERROR: command '%s' not found in the 
schema", command);
+return -1;
+}
+
+if (!arguments)
+arguments = emptyargs = virJSONValueNewObject();
+
+if (virQEMUQAPISchemaPathGet(schemapatharguments, schema, 
&schemarootarguments) < 0 ||
+!schemarootarguments) {
+virBufferAsprintf(debug, "ERROR: failed to look up 'arg-type' of  
'%s'", command);
+return -1;
+}
+
+return testQEMUSchemaValidateRecurse(arguments, schemarootarguments, 
schema, debug);
+}
+
+
 /**
  * testQEMUSchemaGetLatest:
  *
diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h
index 84ee9a9670..e3a375b038 100644
--- a/tests/testutilsqemuschema.h
+++ b/tests/testutilsqemuschema.h
@@ -28,6 +28,12 @@ testQEMUSchemaValidate(virJSONValuePtr obj,
virHashTablePtr schema,
virBufferPtr debug);

+int
+testQEMUSchemaValidateCommand(const char *command,
+  virJSONValuePtr arguments,
+  virHashTablePtr schema,
+  virBufferPtr debug);
+
 virJSONValuePtr
 testQEMUSchemaGetLatest(const char* arch);

-- 
2.26.2

[PATCH 12/15] qemumonitorjsontest: Allow use of deprecated 'query-cpus'

[Peter Krempa]

The command was replaced with 'query-cpus-fast' which is always used
when detected by the capabilities so we can allow our test usage of
the deprecated command even if it will be removed from the schema.

Signed-off-by: Peter Krempa 
---
 tests/qemumonitorjsontest.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 7e0ab4609c..eaaabe9a47 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1440,6 +1440,8 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void 
*opaque)
 if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
 return -1;

+qemuMonitorTestSkipDeprecatedValidation(test, true);
+
 if (qemuMonitorTestAddItem(test, "query-cpus",
"{"
"\"return\": ["
@@ -2696,10 +2698,12 @@ testQemuMonitorCPUInfo(const void *opaque)
queryHotpluggableStr) < 0)
 goto cleanup;

-if (data->fast)
+if (data->fast) {
 queryCpusFunction = "query-cpus-fast";
-else
+} else {
 queryCpusFunction = "query-cpus";
+qemuMonitorTestSkipDeprecatedValidation(test, true);
+}

 if (qemuMonitorTestAddItem(test, queryCpusFunction, queryCpusStr) < 0)
 goto cleanup;
-- 
2.26.2

Re: [libvirt-ci PATCH] vars: Add python3-wheel to the base package dependencies

[Andrea Bolognani]

On Wed, 2020-04-29 at 11:21 +0200, Erik Skultety wrote:
> On platforms where we need to install meson from pip, one will very
> likely see something similar to this when building a container from the
> generated Dockerfile:
> 
> Collecting meson==0.49.0
>   Downloading /meson-0.49.0.tar.gz (1.3MB)
> 100% || 1.3MB 874kB/s
> Building wheels for collected packages: meson
>   Running setup.py bdist_wheel for meson ... error
>   error: invalid command 'bdist_wheel'
>   
>   Failed building wheel for meson
>   Running setup.py clean for meson
> Failed to build meson
> 
> Pip is missing the 'wheel' package necessary to build a wheel from
> sources, if it fails to do that, it falls back to the good old:
> $ setup.py install meson
> which succeeds and no harm was done. However, seeing an error in the
> log always raises eyebrows, so let's fix that very simply by installing
> the 'wheel' package which is available on all supported platforms.
> 
> Signed-off-by: Erik Skultety 
> ---
> Alternatively, we could use --no-cache-dir with pip install, but I'm not sure
> whether it would be enough with new versions of pip. I still feel like
> installing the 'wheel' package explicitly is a more transparent and safe fix
> even though we don't benefit from the resulting meson wheel package inside
> containers at all.
> 
>  guests/vars/mappings.yml  | 4 
>  guests/vars/projects/base.yml | 1 +
>  2 files changed, 5 insertions(+)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt-ci PATCH 2/4] Start building on Fedora 32

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 guests/playbooks/build/jobs/defaults.yml| 2 ++
 guests/playbooks/build/projects/gtk-vnc.yml | 1 +
 guests/playbooks/build/projects/libosinfo.yml   | 1 +
 guests/playbooks/build/projects/libvirt-dbus.yml| 2 ++
 guests/playbooks/build/projects/libvirt-sandbox.yml | 2 ++
 guests/playbooks/build/projects/libvirt-tck.yml | 2 ++
 guests/playbooks/build/projects/libvirt.yml | 1 +
 guests/playbooks/build/projects/osinfo-db-tools.yml | 1 +
 guests/playbooks/build/projects/virt-manager.yml| 3 +++
 guests/playbooks/build/projects/virt-viewer.yml | 1 +
 jenkins/jobs/defaults.yaml  | 2 ++
 jenkins/projects/gtk-vnc.yaml   | 1 +
 jenkins/projects/libosinfo.yaml | 1 +
 jenkins/projects/libvirt-dbus.yaml  | 2 ++
 jenkins/projects/libvirt-sandbox.yaml   | 2 ++
 jenkins/projects/libvirt-tck.yaml   | 2 ++
 jenkins/projects/libvirt.yaml   | 1 +
 jenkins/projects/osinfo-db-tools.yaml   | 1 +
 jenkins/projects/virt-manager.yaml  | 3 +++
 jenkins/projects/virt-viewer.yaml   | 1 +
 20 files changed, 32 insertions(+)

diff --git a/guests/playbooks/build/jobs/defaults.yml 
b/guests/playbooks/build/jobs/defaults.yml
index c521687..363c71d 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -7,6 +7,7 @@ all_machines:
   - libvirt-debian-sid
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
   - libvirt-freebsd-11
   - libvirt-freebsd-12
@@ -19,6 +20,7 @@ rpm_machines:
   - libvirt-centos-8
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
 mingw_machines:
   - libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/gtk-vnc.yml 
b/guests/playbooks/build/projects/gtk-vnc.yml
index 260cf71..47530c7 100644
--- a/guests/playbooks/build/projects/gtk-vnc.yml
+++ b/guests/playbooks/build/projects/gtk-vnc.yml
@@ -16,4 +16,5 @@
   - libvirt-centos-8
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libosinfo.yml 
b/guests/playbooks/build/projects/libosinfo.yml
index 6391323..75795bf 100644
--- a/guests/playbooks/build/projects/libosinfo.yml
+++ b/guests/playbooks/build/projects/libosinfo.yml
@@ -16,4 +16,5 @@
   - libvirt-centos-8
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libvirt-dbus.yml 
b/guests/playbooks/build/projects/libvirt-dbus.yml
index 4cf435d..75ae68a 100644
--- a/guests/playbooks/build/projects/libvirt-dbus.yml
+++ b/guests/playbooks/build/projects/libvirt-dbus.yml
@@ -20,6 +20,7 @@
   - libvirt-debian-sid
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
   - libvirt-opensuse-151
   - libvirt-ubuntu-1804
@@ -31,4 +32,5 @@
   - libvirt-centos-8
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libvirt-sandbox.yml 
b/guests/playbooks/build/projects/libvirt-sandbox.yml
index d9e00d4..dd17d5f 100644
--- a/guests/playbooks/build/projects/libvirt-sandbox.yml
+++ b/guests/playbooks/build/projects/libvirt-sandbox.yml
@@ -10,6 +10,7 @@
   - libvirt-debian-sid
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
   - libvirt-opensuse-151
   - libvirt-ubuntu-1604
@@ -28,4 +29,5 @@
 machines:
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libvirt-tck.yml 
b/guests/playbooks/build/projects/libvirt-tck.yml
index 3a3b277..aa71754 100644
--- a/guests/playbooks/build/projects/libvirt-tck.yml
+++ b/guests/playbooks/build/projects/libvirt-tck.yml
@@ -9,6 +9,7 @@
   - libvirt-debian-sid
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
   - libvirt-freebsd-11
   - libvirt-freebsd-12
@@ -26,4 +27,5 @@
 machines:
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libvirt.yml 
b/guests/playbooks/build/projects/libvirt.yml
index 654d16c..798d87f 100644
--- a/guests/playbooks/build/projects/libvirt.yml
+++ b/guests/playbooks/build/projects/libvirt.yml
@@ -19,6 +19,7 @@
   - libvirt-debian-sid
   - libvirt-fedora-30
   - libvirt-fedora-31
+  - libvirt-fedora-32
   - libvirt-fedora-rawhide
   - libvirt-opensuse-151
   - libvirt-ubuntu-1604
diff -

[libvirt-ci PATCH 4/4] guests: Remove Fedora 30

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 guests/host_vars/libvirt-fedora-30/docker.yml |  2 -
 .../host_vars/libvirt-fedora-30/install.yml   |  2 -
 guests/host_vars/libvirt-fedora-30/main.yml   | 41 ---
 guests/inventory  |  1 -
 4 files changed, 46 deletions(-)
 delete mode 100644 guests/host_vars/libvirt-fedora-30/docker.yml
 delete mode 100644 guests/host_vars/libvirt-fedora-30/install.yml
 delete mode 100644 guests/host_vars/libvirt-fedora-30/main.yml

diff --git a/guests/host_vars/libvirt-fedora-30/docker.yml 
b/guests/host_vars/libvirt-fedora-30/docker.yml
deleted file mode 100644
index 15c78d0..000
--- a/guests/host_vars/libvirt-fedora-30/docker.yml
+++ /dev/null
@@ -1,2 +0,0 @@

-docker_base: fedora:30
diff --git a/guests/host_vars/libvirt-fedora-30/install.yml 
b/guests/host_vars/libvirt-fedora-30/install.yml
deleted file mode 100644
index 7c04f2c..000
--- a/guests/host_vars/libvirt-fedora-30/install.yml
+++ /dev/null
@@ -1,2 +0,0 @@

-install_url: 
https://download.fedoraproject.org/pub/fedora/linux/releases/30/Everything/x86_64/os
diff --git a/guests/host_vars/libvirt-fedora-30/main.yml 
b/guests/host_vars/libvirt-fedora-30/main.yml
deleted file mode 100644
index a8f6fb9..000
--- a/guests/host_vars/libvirt-fedora-30/main.yml
+++ /dev/null
@@ -1,41 +0,0 @@

-projects:
-  - gtk-vnc
-  - libosinfo
-  - libvirt
-  - libvirt-cim
-  - libvirt-dbus
-  - libvirt-glib
-  - libvirt-go
-  - libvirt-go-xml
-  - libvirt-ocaml
-  - libvirt-perl
-  - libvirt-python
-  - libvirt-sandbox
-  - libvirt-tck
-  - osinfo-db
-  - osinfo-db-tools
-  - virt-manager
-  - virt-viewer
-
-os:
-  name: 'Fedora'
-  version: '30'
-
-packaging:
-  format: 'rpm'
-  command: 'dnf'
-
-paths:
-  bash: /bin/bash
-  cc: /usr/bin/gcc
-  ccache: /usr/bin/ccache
-  java: /usr/bin/java
-  make: /usr/bin/make
-  ninja: /usr/bin/ninja
-  python: /usr/bin/python3
-  su: /bin/su
-  sudoers: /etc/sudoers
-
-ansible_python_package: python3
-ansible_python_interpreter: /usr/bin/python3
diff --git a/guests/inventory b/guests/inventory
index c99146e..0a876c8 100644
--- a/guests/inventory
+++ b/guests/inventory
@@ -3,7 +3,6 @@ libvirt-centos-8
 libvirt-debian-9
 libvirt-debian-10
 libvirt-debian-sid
-libvirt-fedora-30
 libvirt-fedora-31
 libvirt-fedora-32
 libvirt-fedora-rawhide
-- 
2.25.4

[libvirt-ci PATCH 1/4] guests: Add Fedora 32

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 guests/host_vars/libvirt-fedora-32/docker.yml |  2 +
 .../host_vars/libvirt-fedora-32/install.yml   |  2 +
 guests/host_vars/libvirt-fedora-32/main.yml   | 41 +++
 guests/inventory  |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 guests/host_vars/libvirt-fedora-32/docker.yml
 create mode 100644 guests/host_vars/libvirt-fedora-32/install.yml
 create mode 100644 guests/host_vars/libvirt-fedora-32/main.yml

diff --git a/guests/host_vars/libvirt-fedora-32/docker.yml 
b/guests/host_vars/libvirt-fedora-32/docker.yml
new file mode 100644
index 000..4523e7c
--- /dev/null
+++ b/guests/host_vars/libvirt-fedora-32/docker.yml
@@ -0,0 +1,2 @@
+---
+docker_base: fedora:32
diff --git a/guests/host_vars/libvirt-fedora-32/install.yml 
b/guests/host_vars/libvirt-fedora-32/install.yml
new file mode 100644
index 000..ec71740
--- /dev/null
+++ b/guests/host_vars/libvirt-fedora-32/install.yml
@@ -0,0 +1,2 @@
+---
+install_url: 
https://download.fedoraproject.org/pub/fedora/linux/releases/32/Everything/x86_64/os
diff --git a/guests/host_vars/libvirt-fedora-32/main.yml 
b/guests/host_vars/libvirt-fedora-32/main.yml
new file mode 100644
index 000..97f5f86
--- /dev/null
+++ b/guests/host_vars/libvirt-fedora-32/main.yml
@@ -0,0 +1,41 @@
+---
+projects:
+  - gtk-vnc
+  - libosinfo
+  - libvirt
+  - libvirt-cim
+  - libvirt-dbus
+  - libvirt-glib
+  - libvirt-go
+  - libvirt-go-xml
+  - libvirt-ocaml
+  - libvirt-perl
+  - libvirt-python
+  - libvirt-sandbox
+  - libvirt-tck
+  - osinfo-db
+  - osinfo-db-tools
+  - virt-manager
+  - virt-viewer
+
+os:
+  name: 'Fedora'
+  version: '32'
+
+packaging:
+  format: 'rpm'
+  command: 'dnf'
+
+paths:
+  bash: /bin/bash
+  cc: /usr/bin/gcc
+  ccache: /usr/bin/ccache
+  java: /usr/bin/java
+  make: /usr/bin/make
+  ninja: /usr/bin/ninja
+  python: /usr/bin/python3
+  su: /bin/su
+  sudoers: /etc/sudoers
+
+ansible_python_package: python3
+ansible_python_interpreter: /usr/bin/python3
diff --git a/guests/inventory b/guests/inventory
index f062310..c99146e 100644
--- a/guests/inventory
+++ b/guests/inventory
@@ -5,6 +5,7 @@ libvirt-debian-10
 libvirt-debian-sid
 libvirt-fedora-30
 libvirt-fedora-31
+libvirt-fedora-32
 libvirt-fedora-rawhide
 libvirt-freebsd-11
 libvirt-freebsd-12
-- 
2.25.4

[libvirt-ci PATCH 0/4] Add Fedora 32, drop Fedora 30

[Andrea Bolognani]

Fedora 32 was released earlier this week, and Fedora 30 goes EOL
the next one.

Andrea Bolognani (4):
  guests: Add Fedora 32
  Start building on Fedora 32
  Stop building on Fedora 30
  guests: Remove Fedora 30

 guests/host_vars/libvirt-fedora-30/docker.yml |2 -
 guests/host_vars/libvirt-fedora-32/docker.yml |2 +
 .../install.yml   |2 +-
 .../main.yml  |2 +-
 guests/inventory  |2 +-
 guests/playbooks/build/jobs/defaults.yml  |4 +-
 guests/playbooks/build/projects/gtk-vnc.yml   |2 +-
 guests/playbooks/build/projects/libosinfo.yml |2 +-
 .../playbooks/build/projects/libvirt-dbus.yml |4 +-
 .../build/projects/libvirt-sandbox.yml|4 +-
 .../playbooks/build/projects/libvirt-tck.yml  |4 +-
 guests/playbooks/build/projects/libvirt.yml   |2 +-
 .../build/projects/osinfo-db-tools.yml|2 +-
 .../playbooks/build/projects/virt-manager.yml |6 +-
 .../playbooks/build/projects/virt-viewer.yml  |2 +-
 jenkins/jobs/defaults.yaml|4 +-
 jenkins/new.xml   | 6423 
 jenkins/old.xml   | 6435 +
 jenkins/projects/gtk-vnc.yaml |2 +-
 jenkins/projects/libosinfo.yaml   |2 +-
 jenkins/projects/libvirt-dbus.yaml|4 +-
 jenkins/projects/libvirt-sandbox.yaml |4 +-
 jenkins/projects/libvirt-tck.yaml |4 +-
 jenkins/projects/libvirt.yaml |2 +-
 jenkins/projects/osinfo-db-tools.yaml |2 +-
 jenkins/projects/virt-manager.yaml|6 +-
 jenkins/projects/virt-viewer.yaml |2 +-
 27 files changed, 12895 insertions(+), 37 deletions(-)
 delete mode 100644 guests/host_vars/libvirt-fedora-30/docker.yml
 create mode 100644 guests/host_vars/libvirt-fedora-32/docker.yml
 rename guests/host_vars/{libvirt-fedora-30 => libvirt-fedora-32}/install.yml 
(66%)
 rename guests/host_vars/{libvirt-fedora-30 => libvirt-fedora-32}/main.yml (97%)
 create mode 100644 jenkins/new.xml
 create mode 100644 jenkins/old.xml

-- 
2.25.4

[libvirt-jenkins-ci PATCH] guests: Run libvirt-dbus test suite on Fedora 31

[Andrea Bolognani]

This was left out by mistake.

Fixes: f79cd9cc250ac32d7e343c15b4697e931a68
Signed-off-by: Andrea Bolognani 
---
Pushed as trivial.

 guests/playbooks/build/projects/libvirt-dbus.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/guests/playbooks/build/projects/libvirt-dbus.yml 
b/guests/playbooks/build/projects/libvirt-dbus.yml
index e29d74b..4cf435d 100644
--- a/guests/playbooks/build/projects/libvirt-dbus.yml
+++ b/guests/playbooks/build/projects/libvirt-dbus.yml
@@ -19,6 +19,7 @@
   - libvirt-debian-10
   - libvirt-debian-sid
   - libvirt-fedora-30
+  - libvirt-fedora-31
   - libvirt-fedora-rawhide
   - libvirt-opensuse-151
   - libvirt-ubuntu-1804
-- 
2.25.4

[jenkins-ci PATCH] jenkins: Fix machine order for libvirt-sandbox

[Andrea Bolognani]

Fixes: f79cd9cc250ac32d7e343c15b4697e931a68
Signed-off-by: Andrea Bolognani 
---
Pushed as trivial.

 jenkins/projects/libvirt-sandbox.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jenkins/projects/libvirt-sandbox.yaml 
b/jenkins/projects/libvirt-sandbox.yaml
index 6a8acec..10c2441 100644
--- a/jenkins/projects/libvirt-sandbox.yaml
+++ b/jenkins/projects/libvirt-sandbox.yaml
@@ -25,5 +25,5 @@
   parent_jobs: 'libvirt-sandbox-check'
   machines:
 - libvirt-fedora-30
-- libvirt-fedora-rawhide
 - libvirt-fedora-31
+- libvirt-fedora-rawhide
-- 
2.25.4

Re: [PATCH] docs: Describe protected virtualization guest setup

[Boris Fiuczynski]

On 4/29/20 3:29 PM, Daniel P. Berrangé wrote:

On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 

Protected virtualization/IBM Secure Execution for Linux protects
guest memory and state from the host.

Add some basic information about technology and a brief guide
on setting up secure guests with libvirt.

Signed-off-by: Viktor Mihajlovski 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Paulo de Rezende Pinatti 
---
  docs/kbase.html.in  |   3 +
  docs/kbase/protected_virtualization.rst | 188 


I'd suggest calling this  s390_protected_virt.rst

Done




diff --git a/docs/kbase.html.in b/docs/kbase.html.in
index c586e0f676..05a3239224 100644
--- a/docs/kbase.html.in
+++ b/docs/kbase.html.in
@@ -14,6 +14,9 @@
  Secure usage
  Secure usage of the libvirt APIs
  
+Protected virtualization


"s390 Protected virtualization"  as the title


As discussed changed to "Protected virtualization on s390".




+Running secure guests with IBM Secure Execution


s/secure guests/secure s390 guests/


Done




+
  Launch security
  Securely launching VMs with AMD SEV
  
diff --git a/docs/kbase/protected_virtualization.rst b/docs/kbase/protected_virtualization.rst

new file mode 100644
index 00..48f2add14e
--- /dev/null
+++ b/docs/kbase/protected_virtualization.rst
@@ -0,0 +1,188 @@
+
+Protected Virtualization


s/^/s390/


As discussed changed to "Protected virtualization on s390".




+



+Host Requirements
+=
+
+IBM Secure Execution for Linux has some hardware and firmware
+requirements. The system hardware must be an IBM z15 (or newer),
+or an IBM LinuxONE III (or newer).
+
+It is also necessary that the IBM Secure Execution feature is
+enabled for that system. Check for facility '158', e.g.
+
+::
+
+   $ grep facilities /proc/cpuinfo | grep 158


I'd suggest that "virt-host-validate" be probing for this
so we can just tell them to run that command.


Extending virt-host-validate can be done.
This documentation follows the same bare bone approach as is used in 
docs/kbase/launch_security_sev.rst.

Would it be OK to explain both with and without libvirt means?

I will have a look at virt-host-validate once the cache invalidation is 
ready.





+The kernel must include the protected virtualization support
+which can be verified by checking for the presence of directory
+``/sys/firmware/uv``. It will only be present when both the
+hardware and the kernel support are available.
+
+Finally, the host operating system must donate some memory to
+the ultravisor needed to store memory security information.
+This is achieved by specifying the following kernel command
+line parameter to the host boot configuration
+
+::
+
+   prot_virt=1
+
+
+Guest Requirements
+==
+
+Guest Boot
+--
+
+To start a guest in protected virtualization secure mode, the
+boot image must have been prepared first with the program
+``genprotimg`` using the correct public key for this host.
+``genprotimg`` is part of the package ``s390-tools``, or
+``s390-utils``, depending on the Linux distribution being used.
+It can also be found at
+``_
+
+The guests have to be configured to use the host CPU model, which
+must contain the ``unpack`` facility indicating ultravisor guest support.
+
+With the following command it's possible to check whether the host
+CPU model satisfies the requirement
+
+::
+
+   $ virsh domcapabilities | grep unpack
+
+which should return
+
+::
+
+   
+
+If the check fails despite the host system actually supporting
+protected virtualization guests, this can be caused by a stale
+libvirt capabilities cache. To recover, run the following
+commands
+
+::
+
+   $ systemctl stop libvirtd
+   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
+   $ systemctl start libvirtd


If this is truly needed that it is a bug in libvirt - we're missing
something in the cache invalidation logic.


Yes, as I wrote to your other mail... we are working on it.




Regards,
Daniel




--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Daniel P . Berrangé]

On Wed, Apr 29, 2020 at 05:50:58PM +0200, Boris Fiuczynski wrote:
> On 4/29/20 5:15 PM, Daniel P. Berrangé wrote:
> > On Wed, Apr 29, 2020 at 05:08:31PM +0200, Viktor Mihajlovski wrote:
> > > 
> > > 
> > > On 4/29/20 3:29 PM, Daniel P. Berrangé wrote:
> > > > On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:
> > > > > From: Viktor Mihajlovski 
> > > > > 
> > > > > Protected virtualization/IBM Secure Execution for Linux protects
> > > > > guest memory and state from the host.
> > > > > 
> > > > > Add some basic information about technology and a brief guide
> > > > > on setting up secure guests with libvirt.
> > > > > 
> > > > > Signed-off-by: Viktor Mihajlovski 
> > > > > Reviewed-by: Boris Fiuczynski 
> > > > > Reviewed-by: Paulo de Rezende Pinatti 
> > > > > ---
> > > > >docs/kbase.html.in  |   3 +
> > > > >docs/kbase/protected_virtualization.rst | 188 
> > > > > 
> > > > 
> > > > I'd suggest calling this  s390_protected_virt.rst
> > > We can do that.
> > > > 
> > > > > diff --git a/docs/kbase.html.in b/docs/kbase.html.in
> > > > > index c586e0f676..05a3239224 100644
> > > > > --- a/docs/kbase.html.in
> > > > > +++ b/docs/kbase.html.in
> > > > > @@ -14,6 +14,9 @@
> > > > >Secure usage
> > > > >Secure usage of the libvirt APIs
> > > > > +Protected 
> > > > > virtualization
> > > > 
> > > > "s390 Protected virtualization"  as the title
> > > > 
> > > The terminology that was used in the KVM upstream code is simply protected
> > > virtualization without a prefix, so I'd avoid creating a new denomination 
> > > in
> > > libvirt.
> > 
> > Putting an "s390" prefix on this isn't inventing new terminology - it is
> > just making it obvious to users what target it applies to.
> 
> Daniel & Viktor: Would calling it "Protected virtualization on s390" serve
> both of you?

Sure.


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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Viktor Mihajlovski]

On 4/29/20 5:50 PM, Boris Fiuczynski wrote:

On 4/29/20 5:15 PM, Daniel P. Berrangé wrote:

On Wed, Apr 29, 2020 at 05:08:31PM +0200, Viktor Mihajlovski wrote:



On 4/29/20 3:29 PM, Daniel P. Berrangé wrote:

On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 

Protected virtualization/IBM Secure Execution for Linux protects
guest memory and state from the host.

Add some basic information about technology and a brief guide
on setting up secure guests with libvirt.

Signed-off-by: Viktor Mihajlovski 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Paulo de Rezende Pinatti 
---
   docs/kbase.html.in  |   3 +
   docs/kbase/protected_virtualization.rst | 188 



I'd suggest calling this  s390_protected_virt.rst

We can do that.



diff --git a/docs/kbase.html.in b/docs/kbase.html.in
index c586e0f676..05a3239224 100644
--- a/docs/kbase.html.in
+++ b/docs/kbase.html.in
@@ -14,6 +14,9 @@
   Secure usage
   Secure usage of the libvirt APIs
+    href="kbase/protected_virtualization.html">Protected 
virtualization


"s390 Protected virtualization"  as the title

The terminology that was used in the KVM upstream code is simply 
protected
virtualization without a prefix, so I'd avoid creating a new 
denomination in

libvirt.


Putting an "s390" prefix on this isn't inventing new terminology - it is
just making it obvious to users what target it applies to.


Daniel & Viktor: Would calling it "Protected virtualization on s390" 
serve both of you?



I can live with that.



Alternatively we could use the (unmodified) marketing name "IBM Secure
Execution for Linux" here and below in the RST and reverse the "also 
known

as" sentence in the overview.


That's even worse IMHO. Just put an s390 prefix on the current text.

Regards,
Daniel






--
Kind Regards,
   Viktor

Re: [PATCH] docs: Describe protected virtualization guest setup

[Boris Fiuczynski]

On 4/29/20 5:15 PM, Daniel P. Berrangé wrote:

On Wed, Apr 29, 2020 at 05:08:31PM +0200, Viktor Mihajlovski wrote:



On 4/29/20 3:29 PM, Daniel P. Berrangé wrote:

On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 

Protected virtualization/IBM Secure Execution for Linux protects
guest memory and state from the host.

Add some basic information about technology and a brief guide
on setting up secure guests with libvirt.

Signed-off-by: Viktor Mihajlovski 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Paulo de Rezende Pinatti 
---
   docs/kbase.html.in  |   3 +
   docs/kbase/protected_virtualization.rst | 188 


I'd suggest calling this  s390_protected_virt.rst

We can do that.



diff --git a/docs/kbase.html.in b/docs/kbase.html.in
index c586e0f676..05a3239224 100644
--- a/docs/kbase.html.in
+++ b/docs/kbase.html.in
@@ -14,6 +14,9 @@
   Secure usage
   Secure usage of the libvirt APIs
+Protected 
virtualization


"s390 Protected virtualization"  as the title


The terminology that was used in the KVM upstream code is simply protected
virtualization without a prefix, so I'd avoid creating a new denomination in
libvirt.


Putting an "s390" prefix on this isn't inventing new terminology - it is
just making it obvious to users what target it applies to.


Daniel & Viktor: Would calling it "Protected virtualization on s390" 
serve both of you?





Alternatively we could use the (unmodified) marketing name "IBM Secure
Execution for Linux" here and below in the RST and reverse the "also known
as" sentence in the overview.


That's even worse IMHO. Just put an s390 prefix on the current text.

Regards,
Daniel




--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

Re: [PATCH V3 5/5] cpu: Introduce getHost support for ARM CPU driver

[Jiri Denemark]

Hi.

On Wed, Apr 29, 2020 at 16:03:19 +0800, Zhenyu Zheng wrote:
> Hi Jiri，
> 
> Thanks alot for the help, I've updated the series to v4 and also attached
> pipeline results for each patch as suggested.

I explicitly said you don't have to send a new version just for that
small issue...

And I don't think pipeline results are needed for each patch, one result
for the whole series should be enough to assure the code compiles on all
tested architectures. Reviewers will usually check themselves whether
the series can be compiled after each patch. I just wanted you to take
the branch with all the patches and push it to gitlab, which will start
a single CI pipeline for all the changes (just like in your v3), but the
branch on gitlab will contain each patch of your series as is (while in
v3 you squashed all patches into a single commit and pushed that).

That said, all this is mainly a guidance for future submission. It's
*not* a request for yet another respin of this series.

In the meantime I found two AArch64 hosts with CPUs recognized by your
changes so I'm playing with them.

Jirka

Re: [PATCH] docs: Describe protected virtualization guest setup

[Daniel P . Berrangé]

On Wed, Apr 29, 2020 at 05:08:31PM +0200, Viktor Mihajlovski wrote:
> 
> 
> On 4/29/20 3:29 PM, Daniel P. Berrangé wrote:
> > On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:
> > > From: Viktor Mihajlovski 
> > > 
> > > Protected virtualization/IBM Secure Execution for Linux protects
> > > guest memory and state from the host.
> > > 
> > > Add some basic information about technology and a brief guide
> > > on setting up secure guests with libvirt.
> > > 
> > > Signed-off-by: Viktor Mihajlovski 
> > > Reviewed-by: Boris Fiuczynski 
> > > Reviewed-by: Paulo de Rezende Pinatti 
> > > ---
> > >   docs/kbase.html.in  |   3 +
> > >   docs/kbase/protected_virtualization.rst | 188 
> > 
> > I'd suggest calling this  s390_protected_virt.rst
> We can do that.
> > 
> > > diff --git a/docs/kbase.html.in b/docs/kbase.html.in
> > > index c586e0f676..05a3239224 100644
> > > --- a/docs/kbase.html.in
> > > +++ b/docs/kbase.html.in
> > > @@ -14,6 +14,9 @@
> > >   Secure usage
> > >   Secure usage of the libvirt APIs
> > > +Protected 
> > > virtualization
> > 
> > "s390 Protected virtualization"  as the title
> > 
> The terminology that was used in the KVM upstream code is simply protected
> virtualization without a prefix, so I'd avoid creating a new denomination in
> libvirt.

Putting an "s390" prefix on this isn't inventing new terminology - it is
just making it obvious to users what target it applies to.

> Alternatively we could use the (unmodified) marketing name "IBM Secure
> Execution for Linux" here and below in the RST and reverse the "also known
> as" sentence in the overview.

That's even worse IMHO. Just put an s390 prefix on the current text.

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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Viktor Mihajlovski]

On 4/29/20 3:29 PM, Daniel P. Berrangé wrote:

On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 

Protected virtualization/IBM Secure Execution for Linux protects
guest memory and state from the host.

Add some basic information about technology and a brief guide
on setting up secure guests with libvirt.

Signed-off-by: Viktor Mihajlovski 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Paulo de Rezende Pinatti 
---
  docs/kbase.html.in  |   3 +
  docs/kbase/protected_virtualization.rst | 188 


I'd suggest calling this  s390_protected_virt.rst

We can do that.



diff --git a/docs/kbase.html.in b/docs/kbase.html.in
index c586e0f676..05a3239224 100644
--- a/docs/kbase.html.in
+++ b/docs/kbase.html.in
@@ -14,6 +14,9 @@
  Secure usage
  Secure usage of the libvirt APIs
  
+Protected virtualization


"s390 Protected virtualization"  as the title

The terminology that was used in the KVM upstream code is simply 
protected virtualization without a prefix, so I'd avoid creating a new 
denomination in libvirt.
Alternatively we could use the (unmodified) marketing name "IBM Secure 
Execution for Linux" here and below in the RST and reverse the "also 
known as" sentence in the overview.

+Running secure guests with IBM Secure Execution


s/secure guests/secure s390 guests/OK


+
  Launch security
  Securely launching VMs with AMD SEV
  
diff --git a/docs/kbase/protected_virtualization.rst b/docs/kbase/protected_virtualization.rst

new file mode 100644
index 00..48f2add14e
--- /dev/null
+++ b/docs/kbase/protected_virtualization.rst
@@ -0,0 +1,188 @@
+
+Protected Virtualization


s/^/s390/


see above

[...]
--
Kind Regards,
   Viktor

Re: [PATCH 1/3] Improve blockpull man entry

[Sebastian Mitterle]

> If the example is not something you plan on putting in the man page,
> then that's okay.  I was just pointing out that anything mentioned in
> the man page should also mention the need for proper shell quoting.

Got it, thank you. I'm unsure. No, I wasn't planning on adding an
example. Peter, you initially reviewed this patch, what do you think?
(I checked and I don't see many examples on the manpage in general.)
Personally, I like examples but am unsure about the scope, like how
many examples, which options should be exemplified, etc.


On Wed, Apr 29, 2020 at 4:16 PM Eric Blake  wrote:
>
> On 4/29/20 8:11 AM, Sebastian Mitterle wrote:
> >> We should really encourage users to properly quote their command line to
> > avoid unintentional globbing:
> >
> > Not sure I understand, is this a request to use '"name[i]"' with
> > single and double quotes in the manpage?
>
> One or the other, but not both.
>
> Buggy:
> # virsh blockpull fedora vda vda[1]
>
> Ok:
> # virsh blockpull fedora vda 'vda[1]'
> # virsh blockpull fedora vda "vda[1]"
> # virsh blockpull fedora vda vda\[1]
>
> the point is that any shell example that can misbehave due to globbing
> if underquoted should instead be properly quoted.
> >
> > Please, note that the line you quote is not part of the patch but
> > sample invocations to demonstrate behavior of blockpull command
> > regarding mandatory positional arguments to justify v2 of this patch.
> >
>
> If the example is not something you plan on putting in the man page,
> then that's okay.  I was just pointing out that anything mentioned in
> the man page should also mention the need for proper shell quoting.
>
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>


-- 
Best,
Sebastian

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance

[Daniel P . Berrangé]

On Tue, Mar 31, 2020 at 03:28:20PM +0100, Daniel P. Berrangé wrote:
> Currently when creating a Dockerfile for a container, we include the
> full set of base packages, along with the packages for the project
> itself. If building a Perl binding, this would require us to install
> the base package, libvirt packages and Perl packages. With the use
> of the "--inherit libvirt-fedora-30" arg, it is possible to have a
> dockerfile that only adds the Perl packages, getting everything
> else from the parent image.
> 
> For example, a full Dockerfile for libvirt-go would thus be:
> 
>   FROM libvirt-centos-7:latest
> 
>   RUN yum install -y \
>   golang && \
>   yum autoremove -y && \
>   yum clean all -y
> 
> Note there is no need to set ENV either, as that's inherited from the
> parent container.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  guests/lcitool | 108 -
>  1 file changed, 98 insertions(+), 10 deletions(-)

While this is a conceptually useful feature, I'm not actually intending
to use it anymore. Instead the new libvirt-minimal and libvirt-dist
projets would be used to create a self-contained dockerfile for downstream
components as illustrated with

ci  https://www.redhat.com/archives/libvir-list/2020-April/msg01407.html
python  https://www.redhat.com/archives/libvir-list/2020-April/msg01418.html
perlhttps://www.redhat.com/archives/libvir-list/2020-April/msg01408.html


It could still be useful if QEMU adopts lcitool since their dockerfiles
currently make use of inheritance

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

[jenkins-ci PATCH] lcitool: Move gtk-vnc MinGW builds to Fedora Rawhide

[Andrea Bolognani]

When moving MinGW builds off Fedora 30, gtk-vnc was mistakenly
left behind.

Fixes: 6ede7a19e7aa1670bbe13b10d8014977d3bb2203
Signed-off-by: Andrea Bolognani 
---
Pushed as a CI fix.

 guests/host_vars/libvirt-fedora-30/main.yml  | 2 --
 guests/host_vars/libvirt-fedora-rawhide/main.yml | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/guests/host_vars/libvirt-fedora-30/main.yml 
b/guests/host_vars/libvirt-fedora-30/main.yml
index 227db89..a8f6fb9 100644
--- a/guests/host_vars/libvirt-fedora-30/main.yml
+++ b/guests/host_vars/libvirt-fedora-30/main.yml
@@ -1,8 +1,6 @@
 ---
 projects:
   - gtk-vnc
-  - gtk-vnc+mingw32
-  - gtk-vnc+mingw64
   - libosinfo
   - libvirt
   - libvirt-cim
diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml 
b/guests/host_vars/libvirt-fedora-rawhide/main.yml
index 2367fe1..bd42a87 100644
--- a/guests/host_vars/libvirt-fedora-rawhide/main.yml
+++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml
@@ -1,6 +1,8 @@
 ---
 projects:
   - gtk-vnc
+  - gtk-vnc+mingw32
+  - gtk-vnc+mingw64
   - libosinfo
   - libosinfo+mingw32
   - libosinfo+mingw64
-- 
2.25.4

Re: [PATCH] docs: Describe protected virtualization guest setup

[Christian Borntraeger]

On 29.04.20 16:09, Christian Borntraeger wrote:
> 
> 
> On 29.04.20 15:25, Daniel P. Berrangé wrote:
>> On Wed, Apr 29, 2020 at 10:19:20AM -0300, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 4/28/20 12:58 PM, Boris Fiuczynski wrote:
 From: Viktor Mihajlovski 

>>>
>>> [...]
 +
 +If the check fails despite the host system actually supporting
 +protected virtualization guests, this can be caused by a stale
 +libvirt capabilities cache. To recover, run the following
 +commands
 +
 +::
 +
 +   $ systemctl stop libvirtd
 +   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
 +   $ systemctl start libvirtd
 +
 +
>>>
>>>
>>> Why isn't Libvirt re-fetching the capabilities after host changes that 
>>> affects
>>> KVM capabilities? I see that we're following up QEMU timestamps to detect
>>> if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
>>> we refresh domain capabilities every time following a host reboot?
>>
>> Caching of capabilities was done precisely  to avoid refreshing on every boot
>> because it resulted in slow startup for apps using libvirt after boot.
> 
> Caching beyond the life time of /dev/kvm seems broken from a kernel 
> perspective.
> It is totally valid to load a new version of the kvm module with new 
> capabilities.
> 
> I am curious, Why did it take so long? we should - on normal system - only 
> refresh _one_ binary as most binaries are just TCG.
> 
> As Boris said, we are going to provide yet another check (besides the nested
> thing.
> 
> But in general I think invalidating the cache for that one and only binary
> that provides KVM after a change of /dev/kvm seems like the proper solution.
> 



Looking into that, I found a handler for the "nested" module parameter. Now: We 
also
have a hpage parameter that decides if you can use large pages or not in the 
host.
Do we need to check that as well or is that something that libvirt does not 
care 
about?
This parameter will go away at a later point in time though as soon as we have 
added
the code to have hpage and nested at the same time. 

So given the shakiness of these things, the time stamp of /dev/kvm really seems 
to be the best way to go forward long term. Would be really interesting to 
understand
the conditions where you saw the long startup times due to rescanning.

Re: [PATCH 1/3] Improve blockpull man entry

[Eric Blake]

On 4/29/20 8:11 AM, Sebastian Mitterle wrote:

We should really encourage users to properly quote their command line to

avoid unintentional globbing:

Not sure I understand, is this a request to use '"name[i]"' with
single and double quotes in the manpage?


One or the other, but not both.

Buggy:
# virsh blockpull fedora vda vda[1]

Ok:
# virsh blockpull fedora vda 'vda[1]'
# virsh blockpull fedora vda "vda[1]"
# virsh blockpull fedora vda vda\[1]

the point is that any shell example that can misbehave due to globbing 
if underquoted should instead be properly quoted.


Please, note that the line you quote is not part of the patch but
sample invocations to demonstrate behavior of blockpull command
regarding mandatory positional arguments to justify v2 of this patch.



If the example is not something you plan on putting in the man page, 
then that's okay.  I was just pointing out that anything mentioned in 
the man page should also mention the need for proper shell quoting.




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

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Eric Blake]

[meta-comment]

On 4/29/20 4:35 AM, Yan Zhao wrote:

On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote:

[...]

This patchset introduces a migration_version attribute under sysfs

of VFIO

Mediated devices.


Hmm, several pages with up to 16 levels of quoting, with editors making 
the lines ragged, all before I get to the real meat of the email. 
Remember, it's okay to trim content,...



So why don't we split the difference; lets say that it should start with
the hex PCI Vendor ID.


The problem is for mdev devices, if the parent devices are not PCI devices,
they don't have PCI vendor IDs.


...to just what you are replying to.

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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Christian Borntraeger]

On 29.04.20 15:25, Daniel P. Berrangé wrote:
> On Wed, Apr 29, 2020 at 10:19:20AM -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 4/28/20 12:58 PM, Boris Fiuczynski wrote:
>>> From: Viktor Mihajlovski 
>>>
>>
>> [...]
>>> +
>>> +If the check fails despite the host system actually supporting
>>> +protected virtualization guests, this can be caused by a stale
>>> +libvirt capabilities cache. To recover, run the following
>>> +commands
>>> +
>>> +::
>>> +
>>> +   $ systemctl stop libvirtd
>>> +   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
>>> +   $ systemctl start libvirtd
>>> +
>>> +
>>
>>
>> Why isn't Libvirt re-fetching the capabilities after host changes that 
>> affects
>> KVM capabilities? I see that we're following up QEMU timestamps to detect
>> if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
>> we refresh domain capabilities every time following a host reboot?
> 
> Caching of capabilities was done precisely  to avoid refreshing on every boot
> because it resulted in slow startup for apps using libvirt after boot.

Caching beyond the life time of /dev/kvm seems broken from a kernel perspective.
It is totally valid to load a new version of the kvm module with new 
capabilities.

I am curious, Why did it take so long? we should - on normal system - only 
refresh _one_ binary as most binaries are just TCG.

As Boris said, we are going to provide yet another check (besides the nested
thing.

But in general I think invalidating the cache for that one and only binary
that provides KVM after a change of /dev/kvm seems like the proper solution.


> 
> We look for specific features that change as a way to indicate a refresh
> is needed.  If there's a need to delete the capabilities manually that
> indicates we're missing some feature when deciding whether the cache is
> stale.

[libvirt-python PATCH 1/3] test: workaround missing VIR_TYPED_PARAM enums in API definition

[Daniel P . Berrangé]

On Ubuntu 1804 with libvirt 4.0.0 libvirt-python build fails

running test
/usr/bin/python3 sanitytest.py build/lib.linux-x86_64-3.6 
/usr/share/libvirt/api/libvirt-api.xml
Cannot get a value of enum VIR_TYPED_PARAM_BOOLEAN (originally 
VIR_DOMAIN_BLKIO_PARAM_BOOLEAN)
Cannot get a value of enum VIR_TYPED_PARAM_DOUBLE (originally 
VIR_DOMAIN_BLKIO_PARAM_DOUBLE)
Cannot get a value of enum VIR_TYPED_PARAM_INT (originally 
VIR_DOMAIN_BLKIO_PARAM_INT)
...snip...

The code generated for the binding is still correct and so we can just
whitelist this error scenario.

Signed-off-by: Daniel P. Berrangé 
---
 sanitytest.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sanitytest.py b/sanitytest.py
index 1bedd55..f187c15 100644
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -73,7 +73,11 @@ for n in second_pass:
 val = int(v[val])
 break
 
-if type(val) != int:
+# Version 4.0.0 was broken as missing VIR_TYPED_PARAM enums
+# constants. This is harmless from POV of validating API
+# coverage so ignore this error.
+if (type(val) != int and
+not val.startswith("VIR_TYPED_PARAM_")):
 fail = True
 print("Cannot get a value of enum %s (originally %s)" % (val, name))
 enumvals[typ][name] = val
-- 
2.25.4

[libvirt-python PATCH 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

[Daniel P . Berrangé]

The python build needs to validate two axis

 - A variety of libvirt versions
 - A variety of python versions

We get coverage for both these axis by running a build against the
distro provided libvirt packages. All that is then missing is a build
against the latest libvirt git master, which only needs to be run on
a single distro, for which CentOS 8 is picked as a stable long life
base.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.yml| 172 ++
 ci/libvirt-centos-7.dkr   |  85 +
 ci/libvirt-centos-8.dkr   |  64 +
 ci/libvirt-debian-10.dkr  |  55 +++
 ci/libvirt-debian-9.dkr   |  58 
 ci/libvirt-debian-sid.dkr |  55 +++
 ci/libvirt-fedora-30.dkr  |  52 ++
 ci/libvirt-fedora-31.dkr  |  52 ++
 ci/libvirt-fedora-rawhide.dkr |  53 +++
 ci/libvirt-opensuse-151.dkr   |  54 +++
 ci/libvirt-ubuntu-1604.dkr|  58 
 ci/libvirt-ubuntu-1804.dkr|  58 
 ci/refresh|  21 +
 13 files changed, 837 insertions(+)
 create mode 100644 .gitlab-ci.yml
 create mode 100644 ci/libvirt-centos-7.dkr
 create mode 100644 ci/libvirt-centos-8.dkr
 create mode 100644 ci/libvirt-debian-10.dkr
 create mode 100644 ci/libvirt-debian-9.dkr
 create mode 100644 ci/libvirt-debian-sid.dkr
 create mode 100644 ci/libvirt-fedora-30.dkr
 create mode 100644 ci/libvirt-fedora-31.dkr
 create mode 100644 ci/libvirt-fedora-rawhide.dkr
 create mode 100644 ci/libvirt-opensuse-151.dkr
 create mode 100644 ci/libvirt-ubuntu-1604.dkr
 create mode 100644 ci/libvirt-ubuntu-1804.dkr
 create mode 100755 ci/refresh

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 000..bfcd31c
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,172 @@
+
+stages:
+  - prebuild
+  - containers
+  - build
+  - docs
+
+.container_job_template: &container_job_definition
+  image: docker:stable
+  stage: containers
+  services:
+- docker:dind
+  before_script:
+- export TAG="${CI_REGISTRY_IMAGE}/buildenv-${NAME}:latest"
+- export 
COMMON_TAG="${CI_REGISTRY}/libvirt/libvirt-perl/buildenv-${NAME}:latest"
+- docker info
+- docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p 
${CI_REGISTRY_PASSWORD}
+  script:
+- docker pull ${TAG} || docker pull ${COMMON_TAG} || true
+- docker build --cache-from ${TAG} --cache-from ${COMMON_TAG} --tag ${TAG} 
-f ci/libvirt-${NAME}.dkr ci
+- docker push ${TAG}
+  after_script:
+- docker logout
+
+.build_git_job_template: &build_git_job_definition
+  stage: build
+  before_script:
+- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)"
+- export SCRATCH_DIR="/tmp/scratch"
+- export VROOT="${SCRATCH_DIR}/vroot"
+- export LD_LIBRARY_PATH="${VROOT}/lib"
+- export PATH="${PATH}:${VROOT}/bin"
+- export PKG_CONFIG_PATH="${VROOT}/lib/pkgconfig"
+  script:
+- pushd ${PWD}
+- mkdir -p ${SCRATCH_DIR}
+- cd ${SCRATCH_DIR}
+- git clone --depth 1 https://gitlab.com/libvirt/libvirt.git src
+- mkdir build
+- cd build
+- ../src/autogen.sh --prefix=${VROOT} --without-libvirtd
+- make install
+- popd
+- ${PYTHON} setup.py build
+- ${PYTHON} setup.py test
+
+.build_dist_job_template: &build_dist_job_definition
+  stage: build
+  before_script:
+- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)"
+- export TEST_MAINTAINER=1
+  script:
+- ${PYTHON} setup.py build
+- ${PYTHON} setup.py test
+
+# Check that all commits are signed-off for the DCO.
+# Skip on "libvirt" namespace, since we only need to run
+# this test on developer's personal forks from which
+# merge requests are submitted
+check-dco:
+  stage: prebuild
+  image: registry.gitlab.com/libvirt/libvirt-ci/check-dco:master
+  script:
+- /check-dco
+  except:
+variables:
+  - $CI_PROJECT_NAMESPACE == 'libvirt'
+
+ctr-centos-7:
+  <<: *container_job_definition
+  variables:
+NAME: centos-7
+
+ctr-centos-8:
+  <<: *container_job_definition
+  variables:
+NAME: centos-8
+
+ctr-debian-9:
+  <<: *container_job_definition
+  variables:
+NAME: debian-9
+
+ctr-debian-10:
+  <<: *container_job_definition
+  variables:
+NAME: debian-10
+
+ctr-debian-sid:
+  <<: *container_job_definition
+  variables:
+NAME: debian-sid
+
+ctr-fedora-30:
+  <<: *container_job_definition
+  variables:
+NAME: fedora-30
+
+ctr-fedora-31:
+  <<: *container_job_definition
+  variables:
+NAME: fedora-31
+
+ctr-fedora-rawhide:
+  <<: *container_job_definition
+  variables:
+NAME: fedora-rawhide
+
+ctr-opensuse-151:
+  <<: *container_job_definition
+  variables:
+NAME: opensuse-151
+
+ctr-ubuntu-1604:
+  <<: *container_job_definition
+  variables:
+NAME: ubuntu-1604
+
+ctr-ubuntu-1804:
+  <<: *container_job_definition
+  variables:
+NAME: ubuntu-1804
+
+
+build-git-centos-8:
+  <<: *build_git_job_definition
+  image: ${CI_REGISTRY_IMAGE}/builden

[libvirt-python PATCH 0/3] gitlab: introduce CI jobs for validating python build

[Daniel P . Berrangé]

This introduces GitLab CI to the python module. Traditional Jenkins has
validated the python build across all distros, using libvirt git. This
tested one axis - a variety of python versions - but failed to test the
other interesting axis - a variety of libvirt versions.

This new CI setup fixes that mistake validating both axis and in the
process uncovered a bug in Ubuntu 1804.

Daniel P. Berrangé (3):
  test: workaround missing VIR_TYPED_PARAM enums in API definition
  gitlab: introduce CI jobs testing git master & distro libvirt
  travis: delete redundant configuration

 .gitlab-ci.yml| 172 ++
 .travis.yml   |  55 ---
 ci/libvirt-centos-7.dkr   |  85 +
 ci/libvirt-centos-8.dkr   |  64 +
 ci/libvirt-debian-10.dkr  |  55 +++
 ci/libvirt-debian-9.dkr   |  58 
 ci/libvirt-debian-sid.dkr |  55 +++
 ci/libvirt-fedora-30.dkr  |  52 ++
 ci/libvirt-fedora-31.dkr  |  52 ++
 ci/libvirt-fedora-rawhide.dkr |  53 +++
 ci/libvirt-opensuse-151.dkr   |  54 +++
 ci/libvirt-ubuntu-1604.dkr|  58 
 ci/libvirt-ubuntu-1804.dkr|  58 
 ci/refresh|  21 +
 sanitytest.py |   6 +-
 15 files changed, 842 insertions(+), 56 deletions(-)
 create mode 100644 .gitlab-ci.yml
 delete mode 100644 .travis.yml
 create mode 100644 ci/libvirt-centos-7.dkr
 create mode 100644 ci/libvirt-centos-8.dkr
 create mode 100644 ci/libvirt-debian-10.dkr
 create mode 100644 ci/libvirt-debian-9.dkr
 create mode 100644 ci/libvirt-debian-sid.dkr
 create mode 100644 ci/libvirt-fedora-30.dkr
 create mode 100644 ci/libvirt-fedora-31.dkr
 create mode 100644 ci/libvirt-fedora-rawhide.dkr
 create mode 100644 ci/libvirt-opensuse-151.dkr
 create mode 100644 ci/libvirt-ubuntu-1604.dkr
 create mode 100644 ci/libvirt-ubuntu-1804.dkr
 create mode 100755 ci/refresh

-- 
2.25.4

[libvirt-python PATCH 3/3] travis: delete redundant configuration

[Daniel P . Berrangé]

Now that we're standardizing on GitLab CI for both official gating CI
and developer CI, there's no compelling reason to continue to support
Travis CI.

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 55 -
 1 file changed, 55 deletions(-)
 delete mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index fe24957..000
--- a/.travis.yml
+++ /dev/null
@@ -1,55 +0,0 @@
-language: python
-os: linux
-dist: xenial
-
-python:
-  - 3.6
-
-env:
-  - LIBVIRT=1.2.0 EXT=gz
-  - LIBVIRT=2.0.0 EXT=xz
-  - LIBVIRT=3.6.0 EXT=xz
-  - LIBVIRT=4.5.0 EXT=xz
-  - LIBVIRT=5.0.0 EXT=xz
-
-install:
-  - sudo apt-get -qqy build-dep libvirt libxml2-dev
-  - sudo apt-get -qqy install curl
-  - pip install -r requirements-test.txt
-  - curl -O -s https://libvirt.org/sources/libvirt-${LIBVIRT}.tar.${EXT}
-  - tar -xf libvirt-${LIBVIRT}.tar.${EXT}
-  - pushd libvirt-${LIBVIRT}
-  - |
-./configure --prefix=`pwd`/../libvirt-vroot \
---without-libvirtd \
---without-esx \
---without-vbox \
---without-libxl \
---without-xen \
---without-qemu \
---without-lxc \
---without-hyperv \
---without-macvtap \
---disable-werror
-  - make
-  - make install
-  - popd
-
-script:
-  - LD_LIBRARY_PATH=`pwd`/libvirt-vroot/lib 
PKG_CONFIG_PATH=`pwd`/libvirt-vroot/lib/pkgconfig python setup.py build sdist 
test
-
-notifications:
-  irc:
-# The channel name "irc.oftc.net#virt" is encrypted against 
libvirt/libvirt-python
-# to prevent IRC notifications from github forks. This was created using:
-# $ travis encrypt -r "libvirt/libvirt-python" "irc.oftc.net#virt"
-channels:
-  - secure: 
"K4JrbRpz4CHtZ1vjthVwseT8K6INJgjtZethP4DN1jOpm1uC5esbe1Q1qJOfB92JbMcdM6DNjrVg5eyTJj35aD9UoGpTUcPMsYrhlTPHZtfAuLv/at2eB2XRmETlhiXHgI6LizX6gTiwGW5ZHYwGChzumWxu141d/L9harNh9R6z8XH9uJpkNdOAIsJcwS56XGZ74CKsrqF5dK6ZYPIyP+i7gPO67gEWo0oD6TiJKR908fw03ZiXarIFmLRlk4MbHywLRF0byfD0gg2Ht/tDX73+59QXjLKo/GvQecwoU8UuuFRJlyhUfvm1JYYydnS+O7fPJvI0FWlYFY7i76aeVqkARHRpHknFueT6kZADOmiyMLuvdr+gWVuyIdX33vVJtDm4T1OtNMG/wy9EUZUU1vEu+gHhaRkf/O0GkMj0Hac4I14BGyd/Wdhto6zWojFiMEG/HRHey6l15MBQu49QyW/YMyWi/LeBWXuCUgwQ/ij5EPgsn36OxCafV9zMz0oXZskwX6rJGQRZsdgdwYvt2hP3muLaJbwVyT0bGlOJDJieOa/LVKOXPcQm26aGfyMuLgm0//E9v++6W1IDKh6+BNsfTKAwTxlAvJyz6Bns3XuUJUxUz2+uQVSS6S3EwEZUJ+yHDd2F4sX5OP1L7TWIOWFbI4vQK90ZZ7/jgiYQbwo="
-on_success: change
-on_failure: always
-  email:
-# The list name 'libvirt...@redhat.com" is encrypted against 
libvirt/libvirt-python
-# to prevent IRC notifications from github forks. This was created using:
-# $ travis encrypt -r "libvirt/libvirt-python" "libvirt...@redhat.com"
-recipients:
-  - secure: 
"l6TTLcEcXdDEldHE2NgSIdt6a0k99ug3hp2W4IlnqJWJfIk/87nysJtLNrA0va20pPApCa3iJfMq4PUmBGiIIimTN0/KgC7tONDraogXhCbgfZp9Ejy/57TXxygSp4oum2kDw/c5uLnfrFV/xcn1fk6hvH6CD3bVcJPOQ/mc5FSKLqN5UzwqNnMpMTtG9qxCwfXJ/Bdm9fbURfezC7djcYRwRfPUe3TSD0L76G2HnQnSy4RqR3KFSjQHFPnSGM5IbsokbOaFKCyp/pHOt7QomQaY7YAPX/K9O+eP+hkkp6DGADkkumHctcgnMoyxpahf7pNKw9S8JYabH2NwREIq8whbp9Mo+R4rYO2ozroLWHaboYs/pBLrs606ivTwOmWGRCpJdCmmKTiZNyo6MRrwiOM6x+2YHUTMOa2kVheRNzaaxMFzHPW2kZ20bujPhfViJsRYj9flo5GJXJLyjluGZK5RjrguNJeIh8VJNBiSHW37uj7drmNBsqMad+65mf/4xtGITBqhz5Spx5R9UMZbuiJvcm8GasJMMdQ+bCfuWYjF2nZvSvFEr54Ii1YrDp6FKQ8YG1aD1/D8Z0/b3pLd/8Pn+M9yIWyO/Sto5TbSUjxBTmTStuDmtYE5uu1miYebvgJH5MovWPBegYgrfI417kPJgCG3q/R0YcZFMKFfQyo="
-- 
2.25.4

Re: [PATCH] docs: Describe protected virtualization guest setup

[Daniel P . Berrangé]

On Tue, Apr 28, 2020 at 05:58:02PM +0200, Boris Fiuczynski wrote:
> From: Viktor Mihajlovski 
> 
> Protected virtualization/IBM Secure Execution for Linux protects
> guest memory and state from the host.
> 
> Add some basic information about technology and a brief guide
> on setting up secure guests with libvirt.
> 
> Signed-off-by: Viktor Mihajlovski 
> Reviewed-by: Boris Fiuczynski 
> Reviewed-by: Paulo de Rezende Pinatti 
> ---
>  docs/kbase.html.in  |   3 +
>  docs/kbase/protected_virtualization.rst | 188 

I'd suggest calling this  s390_protected_virt.rst

> diff --git a/docs/kbase.html.in b/docs/kbase.html.in
> index c586e0f676..05a3239224 100644
> --- a/docs/kbase.html.in
> +++ b/docs/kbase.html.in
> @@ -14,6 +14,9 @@
>  Secure usage
>  Secure usage of the libvirt APIs
>  
> +Protected 
> virtualization

"s390 Protected virtualization"  as the title

> +Running secure guests with IBM Secure Execution

s/secure guests/secure s390 guests/

> +
>  Launch security
>  Securely launching VMs with AMD SEV
>  
> diff --git a/docs/kbase/protected_virtualization.rst 
> b/docs/kbase/protected_virtualization.rst
> new file mode 100644
> index 00..48f2add14e
> --- /dev/null
> +++ b/docs/kbase/protected_virtualization.rst
> @@ -0,0 +1,188 @@
> +
> +Protected Virtualization

s/^/s390/

> +

> +Host Requirements
> +=
> +
> +IBM Secure Execution for Linux has some hardware and firmware
> +requirements. The system hardware must be an IBM z15 (or newer),
> +or an IBM LinuxONE III (or newer).
> +
> +It is also necessary that the IBM Secure Execution feature is
> +enabled for that system. Check for facility '158', e.g.
> +
> +::
> +
> +   $ grep facilities /proc/cpuinfo | grep 158

I'd suggest that "virt-host-validate" be probing for this
so we can just tell them to run that command.

> +The kernel must include the protected virtualization support
> +which can be verified by checking for the presence of directory
> +``/sys/firmware/uv``. It will only be present when both the
> +hardware and the kernel support are available.
> +
> +Finally, the host operating system must donate some memory to
> +the ultravisor needed to store memory security information.
> +This is achieved by specifying the following kernel command
> +line parameter to the host boot configuration
> +
> +::
> +
> +   prot_virt=1
> +
> +
> +Guest Requirements
> +==
> +
> +Guest Boot
> +--
> +
> +To start a guest in protected virtualization secure mode, the
> +boot image must have been prepared first with the program
> +``genprotimg`` using the correct public key for this host.
> +``genprotimg`` is part of the package ``s390-tools``, or
> +``s390-utils``, depending on the Linux distribution being used.
> +It can also be found at
> +``_
> +
> +The guests have to be configured to use the host CPU model, which
> +must contain the ``unpack`` facility indicating ultravisor guest support.
> +
> +With the following command it's possible to check whether the host
> +CPU model satisfies the requirement
> +
> +::
> +
> +   $ virsh domcapabilities | grep unpack
> +
> +which should return
> +
> +::
> +
> +   
> +
> +If the check fails despite the host system actually supporting
> +protected virtualization guests, this can be caused by a stale
> +libvirt capabilities cache. To recover, run the following
> +commands
> +
> +::
> +
> +   $ systemctl stop libvirtd
> +   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
> +   $ systemctl start libvirtd

If this is truly needed that it is a bug in libvirt - we're missing
something in the cache invalidation logic.

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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Boris Fiuczynski]

On 4/29/20 3:25 PM, Daniel P. Berrangé wrote:

On Wed, Apr 29, 2020 at 10:19:20AM -0300, Daniel Henrique Barboza wrote:



On 4/28/20 12:58 PM, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 



[...]

+
+If the check fails despite the host system actually supporting
+protected virtualization guests, this can be caused by a stale
+libvirt capabilities cache. To recover, run the following
+commands
+
+::
+
+   $ systemctl stop libvirtd
+   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
+   $ systemctl start libvirtd
+
+



Why isn't Libvirt re-fetching the capabilities after host changes that affects
KVM capabilities? I see that we're following up QEMU timestamps to detect
if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
we refresh domain capabilities every time following a host reboot?


Caching of capabilities was done precisely  to avoid refreshing on every boot
because it resulted in slow startup for apps using libvirt after boot.

We look for specific features that change as a way to indicate a refresh
is needed.  If there's a need to delete the capabilities manually that
indicates we're missing some feature when deciding whether the cache is
stale.

Regards,
Daniel



Daniel's,
we will provide a patch serie proposing code for such caps cache 
invalidation triggers for IBM Secure Execution as well as for AMD SEV.

Afterwards we can change the documentation as well.

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Daniel P . Berrangé]

On Wed, Apr 29, 2020 at 10:19:20AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 4/28/20 12:58 PM, Boris Fiuczynski wrote:
> > From: Viktor Mihajlovski 
> > 
> 
> [...]
> > +
> > +If the check fails despite the host system actually supporting
> > +protected virtualization guests, this can be caused by a stale
> > +libvirt capabilities cache. To recover, run the following
> > +commands
> > +
> > +::
> > +
> > +   $ systemctl stop libvirtd
> > +   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
> > +   $ systemctl start libvirtd
> > +
> > +
> 
> 
> Why isn't Libvirt re-fetching the capabilities after host changes that affects
> KVM capabilities? I see that we're following up QEMU timestamps to detect
> if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
> we refresh domain capabilities every time following a host reboot?

Caching of capabilities was done precisely  to avoid refreshing on every boot
because it resulted in slow startup for apps using libvirt after boot.

We look for specific features that change as a way to indicate a refresh
is needed.  If there's a need to delete the capabilities manually that
indicates we're missing some feature when deciding whether the cache is
stale.

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

Re: [PATCH] docs: Describe protected virtualization guest setup

[Daniel Henrique Barboza]

On 4/28/20 12:58 PM, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 



[...]

+
+If the check fails despite the host system actually supporting
+protected virtualization guests, this can be caused by a stale
+libvirt capabilities cache. To recover, run the following
+commands
+
+::
+
+   $ systemctl stop libvirtd
+   $ rm /var/cache/libvirt/qemu/capabilities/*.xml
+   $ systemctl start libvirtd
+
+



Why isn't Libvirt re-fetching the capabilities after host changes that affects
KVM capabilities? I see that we're following up QEMU timestamps to detect
if the binary changes, which is sensible, but what about /dev/kvm? Shouldn't
we refresh domain capabilities every time following a host reboot?

IMHO this is a discussion worth having before making this sort of workaround
an official part of the feature.



Thanks,


DHB

Re: [PATCH 1/3] Improve blockpull man entry

[Sebastian Mitterle]

> We should really encourage users to properly quote their command line to
avoid unintentional globbing:

Not sure I understand, is this a request to use '"name[i]"' with
single and double quotes in the manpage?

Please, note that the line you quote is not part of the patch but
sample invocations to demonstrate behavior of blockpull command
regarding mandatory positional arguments to justify v2 of this patch.

On Tue, Apr 28, 2020 at 4:33 PM Eric Blake  wrote:
>
> On 4/28/20 9:19 AM, Sebastian Mitterle wrote:
> >> So bandwidth indeed is a positional argument. Since all of the manpage
> >> uses the syntax we've had here originally fixing just this place would
> >> be wrong. The only fix that should be done here is to group the --bytes
> >> flag under bandwidth as specifying --bytes is mandatory.
> >
> > I think there's misunderstanding:
> >
> > # virsh blockpull fedora vda vda[1]
>
> We should really encourage users to properly quote their command line to
> avoid unintentional globbing:
>
> # virsh blockpull fedora vda "vda[1]"
>
> (Otherwise, if I 'touch vda1', the unquoted command will see the
> argument 'vda1')
>
> > error: Scaled numeric value 'vda[1]' for <--bandwidth> option is
> > malformed or out of range
> >
> > # virsh blockpull fedora vda 1024 vda[1]
> > Block Pull started
> >
> > I'll change to [bandwidth [--bytes] [base]]
> >
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>


-- 
Best,
Sebastian

Re: [PATCH libvirt v1 0/6] Fix ZPCI address auto-generation on s390

[Shalini Chellathurai Saroja]

Hi Andrea,

Ping, in case you missed it.

On 4/20/20 9:55 PM, Boris Fiuczynski wrote:

On 4/10/20 2:06 PM, Andrea Bolognani wrote:

On Thu, 2020-04-09 at 12:30 +0200, Shalini Chellathurai Saroja wrote:

The ZPCI address validation or autogeneration does not work as
expected in the following scenarios
1. uid = 0 and fid = 0
2. uid = 0 and fid > 0
3. uid = 0 and fid not specified
4. uid not specified and fid > 0
5. 2 ZPCI devices with uid > 0 and fid not specified.

This is because of the following reasons
1. If uid = 0 or fid = 0 the code assumes that user has not specified
    the corresponding address
2. If either uid or fid is provided, the code assumes that both uid
    and fid addresses are specified by the user.


I'd have to dig up the old threads, but based on what I remember the
behaviors you describe are entirely intentional.

For PCI addresses, setting all parts of the address to zero or not
setting it at all is equivalent, and we wanted to be consistent with
that behavior for ZPCI; additionally, zero is not a valid value for
uid so of course neither is the address uid=0 fid=0, which means that
we're not preventing the user from specifying a valid address by
conflating the all-zero address with the unspecified address.

For partially-specified addresses, the behavior is also the same as
PCI: any part you don't specify is considered to be zero, which
results in

   uid=0 fid=0 -> uid=0 fid=0 -> address gets autogenerated
   uid=0 fid=x -> uid=0 fid=x -> address is rejected as invalid
   uid=0   -> uid=0 fid=0 -> address gets autogenerated
 fid=x -> uid=0 fid=x -> address is rejected as invalid
   uid=x   -> uid=x fid=0 -> address is accepted

So, just like for PCI addresses, you have basically two reasonable
options: either don't specify any zPCI address and leave allocation
entirely up to libvirt, or specify all of the addresses completely:
anything in between will likely not work as you'd expect or want.

Again, this is based purely on my recollection of design discussions
that happened one and a half years ago, so I might have gotten some
of the details wrong - in which case by all means call me out on
that O:-)


Hi Andrea,
sorry for the delayed answer. I (and some others as well) lost some 
emails on my IMAP account and I just found your answer today.
I can remember that you had a discussion with the original author of 
the zpci code. There are a few issues with the currently implemented 
"rules" which partially are not even working as you outlined above in 
more complex scenarios.


First: Setting uid=0 or uid='0x'
The architecture allows to do that BUT if you do than you are NOT 
using the uid mode which results for the guest OS to use the legacy 
mode for assigning PCI addresses starting with 0 increasing by one 
following an unpredictable order by which the pci device are presented 
to guest OS. Since we never ever wanted to support the legacy mode in 
KVM guests we decided to never allow uid=0. Allowing the uid to be set 
to 0 is a contradiction.
Actually the user can also set uid='0x' which I consider very 
specific and one would end up with something like uid='0x0001' and 
even more confusing is that suddenly setting uid='0x' on more than 
one PCI device is allowed.


Besides that the current zpci code still contains at least one flaw 
that is simply caused by the fact that there is no knowledge about 
which value was specified by the user.
In Shalini's and your list it is case 5: This scenario runs into 
errors when another PCI device already has a fid set to 0 OR another 
PCI device exists specified with a uid > 0 and without a fid. The user 
gets the error message for something he did not specify:

 error: Failed to define domain from pci-addr-test.xml
 error: internal error: zPCI fid 0 is already reserved

Regarding setting fid=0 or fid='0x'
Since it is a legal value for fid specifying it should not be 
considered as a wildcard or set equivalent to not specifying it at all.
Doing so things like this happen and for the user it certainly seems 
like a bug:

Specify this in the domain:
  pcidev1: uid='0x' fid='0x'
  pcidev2: uid='0x'
Results in a defined domain:
  pcidev1: uid='0x0002' fid='0x0001'
  pcidev2: uid='0x0001' fid='0x'
Another example:
Specify this in the domain:
  pcidev1: fid='0x'
  pcidev2: fid='0x'
Results in a defined domain:
  pcidev1: uid='0x0002' fid='0x0001'
  pcidev2: uid='0x0001' fid='0x'
 BUT
Specify this in the domain:
  pcidev1: uid='0x0002' fid='0x'
  pcidev2: uid='0x0001' fid='0x'
Results in error:
  error: Failed to define domain from pci-addr-test.xml
  error: internal error: zPCI fid 0 is already reserved
(Btw remove one of the fids results in the flaw described above.)


I think that Shalini's patch series improves the zpci address 
generation to better meet the users expected behavior. It also removes 
a correlation between uid and fid that does

[libvirt-perl PATCH v2 1/3] Build: bump min perl to 5.16.0

[Daniel P . Berrangé]

Based on the supported platforms list, the oldest Perl we need to
support is from RHEL-7, version 5.16.0

Signed-off-by: Daniel P. Berrangé 
---
 Build.PL | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Build.PL b/Build.PL
index ca3c786..7764e41 100755
--- a/Build.PL
+++ b/Build.PL
@@ -73,7 +73,7 @@ my $b = Module::Build->new(
 dist_author => 'Daniel Berrange ',
 dist_abstract => 'libvirt Perl API',
 requires => {
-'perl' => '5.8.0',
+'perl' => '5.16.0',
 },
 extra_compiler_flags => $GCC_CFLAGS . $LIBVIRT_CFLAGS,
 extra_linker_flags => $LIBVIRT_LIBS,
-- 
2.25.4

[libvirt-perl PATCH v2 2/3] gitlab: add CI jobs for validating build across platforms

[Daniel P . Berrangé]

This introduces CI jobs that replace the current jobs used on Jenkins
for every platform except FreeBSD.

A merge request workflow requires the user to fork the primary git
repo into their personal namespace. In general the changes need to
be tested against the current libvirt git master. If the user has a
fork of the main libvirt repo, we don't want to use that by default
as it may be out of date.

The general goal is that the CI jobs are self-contained and don't
depend on the build artifacts from the libvirt repo. We also want
to avoid having an explicit dependency on the libvirt-ci repo, or
on the Quay.io service. Contributors to the Perl module need to be
able to make code changes which imply CI environment changes and
be able to test them in isolation.

Thus, the dockerfile recipes for each distro are added in the ci/
sub-directory. The first stage of the CI jobs is to use these
recipes to build and publish a container image. These images are
then used in the second stage to perform the actual build.

The container image build is cached, inheriting from both the
primary libvirt project namespace, and the user's private project
namespace. Thus the performance hit of building container images
will only be felt the first time the project is forked, or when
the parent Docker images are rebuilt.

The dockerfiles were originally generated using lcitool, but if
the user makes a change that introduces new build dependencies,
the corresponding packages can be added to the dockerfile recipes
directly in the same commit. The change can be propagated back
into the libvirt-ci.git repo asynchronously.

The build job will do a minimal(-ish) build of libvirt git master
and then build the rest of the code against that. Ideally the main
libvirt configure script would have a way to request a minimal
build of just the API and test driver, but for now we settle for
just --without-libvirt which culls a large number of the drivers
fairly easily.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.yml| 143 ++
 ci/libvirt-centos-7.dkr   |  97 +++
 ci/libvirt-centos-8.dkr   |  68 
 ci/libvirt-debian-10.dkr  |  68 
 ci/libvirt-debian-9.dkr   |  71 +
 ci/libvirt-debian-sid.dkr |  68 
 ci/libvirt-fedora-30.dkr  |  66 
 ci/libvirt-fedora-31.dkr  |  66 
 ci/libvirt-fedora-rawhide.dkr |  67 
 ci/libvirt-opensuse-151.dkr   |  66 
 ci/libvirt-ubuntu-1604.dkr|  71 +
 ci/libvirt-ubuntu-1804.dkr|  71 +
 ci/refresh|  16 
 13 files changed, 938 insertions(+)
 create mode 100644 ci/libvirt-centos-7.dkr
 create mode 100644 ci/libvirt-centos-8.dkr
 create mode 100644 ci/libvirt-debian-10.dkr
 create mode 100644 ci/libvirt-debian-9.dkr
 create mode 100644 ci/libvirt-debian-sid.dkr
 create mode 100644 ci/libvirt-fedora-30.dkr
 create mode 100644 ci/libvirt-fedora-31.dkr
 create mode 100644 ci/libvirt-fedora-rawhide.dkr
 create mode 100644 ci/libvirt-opensuse-151.dkr
 create mode 100644 ci/libvirt-ubuntu-1604.dkr
 create mode 100644 ci/libvirt-ubuntu-1804.dkr
 create mode 100755 ci/refresh

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 50dae92..4a1472b 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,6 +1,49 @@
 
 stages:
   - prebuild
+  - containers
+  - build
+
+.container_job_template: &container_job_definition
+  image: docker:stable
+  stage: containers
+  services:
+- docker:dind
+  before_script:
+- export TAG="${CI_REGISTRY_IMAGE}/buildenv-${NAME}:latest"
+- export 
COMMON_TAG="${CI_REGISTRY}/libvirt/libvirt-perl/buildenv-${NAME}:latest"
+- docker info
+- docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p 
${CI_REGISTRY_PASSWORD}
+  script:
+- docker pull ${TAG} || docker pull ${COMMON_TAG} || true
+- docker build --cache-from ${TAG} --cache-from ${COMMON_TAG} --tag ${TAG} 
-f ci/libvirt-${NAME}.dkr ci
+- docker push ${TAG}
+  after_script:
+- docker logout
+
+.build_job_template: &build_job_definition
+  stage: build
+  before_script:
+- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)"
+- export SCRATCH_DIR="/tmp/scratch"
+- export VROOT="${SCRATCH_DIR}/vroot"
+- export LD_LIBRARY_PATH="${VROOT}/lib"
+- export PATH="${PATH}:${VROOT}/bin"
+- export PKG_CONFIG_PATH="${VROOT}/lib/pkgconfig"
+- export TEST_MAINTAINER=1
+  script:
+- pushd ${PWD}
+- mkdir -p ${SCRATCH_DIR}
+- cd ${SCRATCH_DIR}
+- git clone --depth 1 https://gitlab.com/libvirt/libvirt.git src
+- mkdir build
+- cd build
+- ../src/autogen.sh --prefix=${VROOT} --without-libvirtd
+- make install
+- popd
+- perl Build.PL
+- perl Build
+- perl Build test
 
 # Check that all commits are signed-off for the DCO.
 # Skip on "libvirt" namespace, since we only need to run
@@

[libvirt-perl PATCH v2 0/3] gitlab: introduce CI coverage

[Daniel P . Berrangé]

This series introduces CI jobs for the Perl binding, enabling the switch
over to use of Merge Requests with pre-merge build validation.

v1: https://www.redhat.com/archives/libvir-list/2020-April/msg01157.html

This is a different approach from v1, because we don't inherit from the
main libvirt container images. Instead we use the libvirt-minimal
project as a dependancy. The cost is that the container build stage is
more expensive, but the main project build stage is cheaper. This is a
net win because the container build stage is cached so is only a penalty
the first time, or when the distro parent image has changes to pull in.
It also makes the CI process of the binding more self-contained avoiding
by avoiding a dep on the container image for libvirt.

Daniel P. Berrangé (3):
  Build: bump min perl to 5.16.0
  gitlab: add CI jobs for validating build across platforms
  gitlab: add a simple job that publishes the API docs as HTML

 .gitlab-ci.yml| 158 ++
 Build.PL  |   2 +-
 ci/libvirt-centos-7.dkr   |  97 +
 ci/libvirt-centos-8.dkr   |  68 +++
 ci/libvirt-debian-10.dkr  |  68 +++
 ci/libvirt-debian-9.dkr   |  71 +++
 ci/libvirt-debian-sid.dkr |  68 +++
 ci/libvirt-fedora-30.dkr  |  66 ++
 ci/libvirt-fedora-31.dkr  |  66 ++
 ci/libvirt-fedora-rawhide.dkr |  67 ++
 ci/libvirt-opensuse-151.dkr   |  66 ++
 ci/libvirt-ubuntu-1604.dkr|  71 +++
 ci/libvirt-ubuntu-1804.dkr|  71 +++
 ci/refresh|  16 
 14 files changed, 954 insertions(+), 1 deletion(-)
 create mode 100644 ci/libvirt-centos-7.dkr
 create mode 100644 ci/libvirt-centos-8.dkr
 create mode 100644 ci/libvirt-debian-10.dkr
 create mode 100644 ci/libvirt-debian-9.dkr
 create mode 100644 ci/libvirt-debian-sid.dkr
 create mode 100644 ci/libvirt-fedora-30.dkr
 create mode 100644 ci/libvirt-fedora-31.dkr
 create mode 100644 ci/libvirt-fedora-rawhide.dkr
 create mode 100644 ci/libvirt-opensuse-151.dkr
 create mode 100644 ci/libvirt-ubuntu-1604.dkr
 create mode 100644 ci/libvirt-ubuntu-1804.dkr
 create mode 100755 ci/refresh

-- 
2.25.4

[libvirt-perl PATCH v2 3/3] gitlab: add a simple job that publishes the API docs as HTML

[Daniel P . Berrangé]

The Perl modules have inline POD docs. This can be converted to HTML and
published as a GitLab artifact. The rendered HTML is kind of ugly but
improving this is left as an exercise for the future.

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

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4a1472b..2fa54f2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -3,6 +3,7 @@ stages:
   - prebuild
   - containers
   - build
+  - docs
 
 .container_job_template: &container_job_definition
   image: docker:stable
@@ -157,3 +158,17 @@ build-ubuntu-1604:
 build-ubuntu-1804:
   <<: *build_job_definition
   image: ${CI_REGISTRY_IMAGE}/buildenv-ubuntu-1804:latest
+
+apiref:
+  stage: docs
+  image: perl
+  script:
+- mkdir apiref
+- perl -MPod::Simple::HTMLBatch -e Pod::Simple::HTMLBatch::go  lib apiref
+  artifacts:
+expose_as: 'API Reference'
+name: 'apiref'
+when: on_success
+expire_in: 30 days
+paths:
+  - apiref
-- 
2.25.4

[libvirt-ci PATCH] guests: introduce libvirt-dist and libvirt-minimal projects

[Daniel P . Berrangé]

The language bindings have traditionally built against a full libvirt,
however, this is serious overkill because all they should really need
most of the time is access to the main API, and the test driver if they
want to run functional tests. The full libvirt build with virt drivers
is only needed for integration testing which most bindings won't do.

Thus this introduces a new project "libvirt-minimal" which lists the
bare minimum dependencies required to build libvirt.

Taking libvirt-perl as an example, if creating a container using the
current "libvirt,libvirt-perl" project set, the result with 1.4 GB
in size for Fedora 31. With the "libvirt-minimal,libvirt-perl" set,
the size is just 777 MB.

Some projects also wish to have the ability to build against the distro
provided libvirt instead of the latest git master, and for this purpose
a "libvirt-dist" project is defined which pulls in the package needed
for building against the distro libvirt.

In the lcitool dockerfile command, there is a check that the requested
project is configured against the requested host. These new projects are
not listed against any host, because we don't want them installed by
default in the VMs. Thus the dockerfile check is no longer valid and is
removed. Whomever is invoking lcitool knows what combination they want
and whether it will work.

Signed-off-by: Daniel P. Berrangé 
---
 guests/lcitool   |  7 ---
 guests/vars/mappings.yml |  5 +
 guests/vars/projects/libvirt-dist.yml|  3 +++
 guests/vars/projects/libvirt-minimal.yml | 13 +
 4 files changed, 21 insertions(+), 7 deletions(-)
 create mode 100644 guests/vars/projects/libvirt-dist.yml
 create mode 100644 guests/vars/projects/libvirt-minimal.yml

diff --git a/guests/lcitool b/guests/lcitool
index 0c89e13..abc87d2 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -1007,13 +1007,6 @@ class Application:
 for project in projects:
 if project.rfind("+mingw") >= 0:
 raise Exception("Obsolete syntax, please use --cross-arch")
-if project not in facts["projects"]:
-raise Exception(
-"Host {} doesn't support project {}".format(
-host,
-project,
-)
-)
 
 varmap = self._dockerfile_build_varmap(facts, mappings, pip_mappings, 
projects, cross_arch)
 self._dockerfile_format(facts, cross_arch, varmap)
diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 753f0fe..a9ab3d5 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -450,6 +450,11 @@ mappings:
 rpm: libuuid-devel
 cross-policy-deb: foreign
 
+  libvirt-devel:
+rpm: libvirt-devel
+deb: libvirt-dev
+pkg: libvirt
+
   libxml2:
 deb: libxml2-dev
 pkg: libxml2
diff --git a/guests/vars/projects/libvirt-dist.yml 
b/guests/vars/projects/libvirt-dist.yml
new file mode 100644
index 000..aa95f23
--- /dev/null
+++ b/guests/vars/projects/libvirt-dist.yml
@@ -0,0 +1,3 @@
+---
+packages:
+  - libvirt-devel
diff --git a/guests/vars/projects/libvirt-minimal.yml 
b/guests/vars/projects/libvirt-minimal.yml
new file mode 100644
index 000..9402c4f
--- /dev/null
+++ b/guests/vars/projects/libvirt-minimal.yml
@@ -0,0 +1,13 @@
+---
+packages:
+  - glib2
+  - gnutls
+  - libnl3
+  - libnlroute3
+  - libpcap
+  - libtirpc
+  - libxml2
+  - python3-docutils
+  - rpcgen
+  - xmllint
+  - xsltproc
-- 
2.25.4

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Dr. David Alan Gilbert]

* Yan Zhao (yan.y.z...@intel.com) wrote:
> On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote:
> > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > On Tue, Apr 28, 2020 at 10:14:37PM +0800, Dr. David Alan Gilbert wrote:
> > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert 
> > > > > wrote:
> > > > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > > > On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert 
> > > > > > > wrote:
> > > > > > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > > > > > On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote:
> > > > > > > > > > > From: Yan Zhao
> > > > > > > > > > > Sent: Tuesday, April 21, 2020 10:37 AM
> > > > > > > > > > > 
> > > > > > > > > > > On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson 
> > > > > > > > > > > wrote:
> > > > > > > > > > > > On Sun, 19 Apr 2020 21:24:57 -0400
> > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia 
> > > > > > > > > > > > > Huck wrote:
> > > > > > > > > > > > > > On Fri, 17 Apr 2020 05:52:02 -0400
> > > > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, 
> > > > > > > > > > > > > > > Cornelia Huck wrote:
> > > > > > > > > > > > > > > > On Mon, 13 Apr 2020 01:52:01 -0400
> > > > > > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > This patchset introduces a migration_version 
> > > > > > > > > > > > > > > > > attribute under sysfs
> > > > > > > > > > > of VFIO
> > > > > > > > > > > > > > > > > Mediated devices.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > This migration_version attribute is used to 
> > > > > > > > > > > > > > > > > check migration
> > > > > > > > > > > compatibility
> > > > > > > > > > > > > > > > > between two mdev devices.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Currently, it has two locations:
> > > > > > > > > > > > > > > > > (1) under mdev_type node,
> > > > > > > > > > > > > > > > > which can be used even before device 
> > > > > > > > > > > > > > > > > creation, but only for
> > > > > > > > > > > mdev
> > > > > > > > > > > > > > > > > devices of the same mdev type.
> > > > > > > > > > > > > > > > > (2) under mdev device node,
> > > > > > > > > > > > > > > > > which can only be used after the mdev 
> > > > > > > > > > > > > > > > > devices are created, but
> > > > > > > > > > > the src
> > > > > > > > > > > > > > > > > and target mdev devices are not 
> > > > > > > > > > > > > > > > > necessarily be of the same
> > > > > > > > > > > mdev type
> > > > > > > > > > > > > > > > > (The second location is newly added in v5, in 
> > > > > > > > > > > > > > > > > order to keep
> > > > > > > > > > > consistent
> > > > > > > > > > > > > > > > > with the migration_version node for 
> > > > > > > > > > > > > > > > > migratable pass-though
> > > > > > > > > > > devices)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > What is the relationship between those two 
> > > > > > > > > > > > > > > > attributes?
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > (1) is for mdev devices specifically, and (2) is 
> > > > > > > > > > > > > > > provided to keep the
> > > > > > > > > > > same
> > > > > > > > > > > > > > > sysfs interface as with non-mdev cases. so (2) is 
> > > > > > > > > > > > > > > for both mdev
> > > > > > > > > > > devices and
> > > > > > > > > > > > > > > non-mdev devices.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > in future, if we enable vfio-pci vendor ops, 
> > > > > > > > > > > > > > > (i.e. a non-mdev device
> > > > > > > > > > > > > > > is binding to vfio-pci, but is able to register 
> > > > > > > > > > > > > > > migration region and do
> > > > > > > > > > > > > > > migration transactions from a vendor provided 
> > > > > > > > > > > > > > > affiliate driver),
> > > > > > > > > > > > > > > the vendor driver would export (2) directly, 
> > > > > > > > > > > > > > > under device node.
> > > > > > > > > > > > > > > It is not able to provide (1) as there're no mdev 
> > > > > > > > > > > > > > > devices involved.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Ok, creating an alternate attribute for non-mdev 
> > > > > > > > > > > > > > devices makes sense.
> > > > > > > > > > > > > > However, wouldn't that rather be a case (3)? The 
> > > > > > > > > > > > > > change here only
> > > > > > > > > > > > > > refers to mdev devices.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > as you pointed below, (3) and (2) serve the same 
> > > > > > > > > > > > > purpose.
> > > > > > > > > > > > > and I think a possible usage is to migrate between a 
> > > > > > > > > > > > > non-mdev device and
> > > > > > > > > > > > > an mdev device. so I think 

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote:
> * Yan Zhao (yan.y.z...@intel.com) wrote:
> > On Tue, Apr 28, 2020 at 10:14:37PM +0800, Dr. David Alan Gilbert wrote:
> > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert wrote:
> > > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > > On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert 
> > > > > > wrote:
> > > > > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > > > > On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote:
> > > > > > > > > > From: Yan Zhao
> > > > > > > > > > Sent: Tuesday, April 21, 2020 10:37 AM
> > > > > > > > > > 
> > > > > > > > > > On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson 
> > > > > > > > > > wrote:
> > > > > > > > > > > On Sun, 19 Apr 2020 21:24:57 -0400
> > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > >
> > > > > > > > > > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck 
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > On Fri, 17 Apr 2020 05:52:02 -0400
> > > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia 
> > > > > > > > > > > > > > Huck wrote:
> > > > > > > > > > > > > > > On Mon, 13 Apr 2020 01:52:01 -0400
> > > > > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > This patchset introduces a migration_version 
> > > > > > > > > > > > > > > > attribute under sysfs
> > > > > > > > > > of VFIO
> > > > > > > > > > > > > > > > Mediated devices.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > This migration_version attribute is used to 
> > > > > > > > > > > > > > > > check migration
> > > > > > > > > > compatibility
> > > > > > > > > > > > > > > > between two mdev devices.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Currently, it has two locations:
> > > > > > > > > > > > > > > > (1) under mdev_type node,
> > > > > > > > > > > > > > > > which can be used even before device 
> > > > > > > > > > > > > > > > creation, but only for
> > > > > > > > > > mdev
> > > > > > > > > > > > > > > > devices of the same mdev type.
> > > > > > > > > > > > > > > > (2) under mdev device node,
> > > > > > > > > > > > > > > > which can only be used after the mdev 
> > > > > > > > > > > > > > > > devices are created, but
> > > > > > > > > > the src
> > > > > > > > > > > > > > > > and target mdev devices are not necessarily 
> > > > > > > > > > > > > > > > be of the same
> > > > > > > > > > mdev type
> > > > > > > > > > > > > > > > (The second location is newly added in v5, in 
> > > > > > > > > > > > > > > > order to keep
> > > > > > > > > > consistent
> > > > > > > > > > > > > > > > with the migration_version node for migratable 
> > > > > > > > > > > > > > > > pass-though
> > > > > > > > > > devices)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > What is the relationship between those two 
> > > > > > > > > > > > > > > attributes?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > (1) is for mdev devices specifically, and (2) is 
> > > > > > > > > > > > > > provided to keep the
> > > > > > > > > > same
> > > > > > > > > > > > > > sysfs interface as with non-mdev cases. so (2) is 
> > > > > > > > > > > > > > for both mdev
> > > > > > > > > > devices and
> > > > > > > > > > > > > > non-mdev devices.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > in future, if we enable vfio-pci vendor ops, (i.e. 
> > > > > > > > > > > > > > a non-mdev device
> > > > > > > > > > > > > > is binding to vfio-pci, but is able to register 
> > > > > > > > > > > > > > migration region and do
> > > > > > > > > > > > > > migration transactions from a vendor provided 
> > > > > > > > > > > > > > affiliate driver),
> > > > > > > > > > > > > > the vendor driver would export (2) directly, under 
> > > > > > > > > > > > > > device node.
> > > > > > > > > > > > > > It is not able to provide (1) as there're no mdev 
> > > > > > > > > > > > > > devices involved.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Ok, creating an alternate attribute for non-mdev 
> > > > > > > > > > > > > devices makes sense.
> > > > > > > > > > > > > However, wouldn't that rather be a case (3)? The 
> > > > > > > > > > > > > change here only
> > > > > > > > > > > > > refers to mdev devices.
> > > > > > > > > > > > >
> > > > > > > > > > > > as you pointed below, (3) and (2) serve the same 
> > > > > > > > > > > > purpose.
> > > > > > > > > > > > and I think a possible usage is to migrate between a 
> > > > > > > > > > > > non-mdev device and
> > > > > > > > > > > > an mdev device. so I think it's better for them both to 
> > > > > > > > > > > > use (2) rather
> > > > > > > > > > > > than creating (3).
> > > > > > > > > > >
> > > > > > > > > > > An mdev type is meant to define a software compatible 
> > > > > > > > > > > i

Re: [libvirt-ci PATCH 0/2] Move MinGW builds back to Fedora Rawhide

[Erik Skultety]

On Wed, Apr 29, 2020 at 10:58:27AM +0200, Andrea Bolognani wrote:
> On Mon, 2020-04-20 at 12:11 +0200, Andrea Bolognani wrote:
> > Andrea Bolognani (2):
> >   lcitool: Prepare Fedora Rawhide for MinGW builds
> >   Move MinGW builds back to Fedora Rawhide
> > 
> >  guests/host_vars/libvirt-fedora-30/main.yml  | 10 --
> >  guests/host_vars/libvirt-fedora-rawhide/main.yml | 10 ++
> >  guests/playbooks/build/jobs/defaults.yml |  2 +-
> >  jenkins/jobs/defaults.yaml   |  2 +-
> >  4 files changed, 12 insertions(+), 12 deletions(-)
> 
> ping
> 
Reviewed-by: Erik Skultety 

[libvirt-ci PATCH] vars: Add python3-wheel to the base package dependencies

[Erik Skultety]

On platforms where we need to install meson from pip, one will very
likely see something similar to this when building a container from the
generated Dockerfile:

Collecting meson==0.49.0
  Downloading /meson-0.49.0.tar.gz (1.3MB)
100% || 1.3MB 874kB/s
Building wheels for collected packages: meson
  Running setup.py bdist_wheel for meson ... error
  error: invalid command 'bdist_wheel'
  
  Failed building wheel for meson
  Running setup.py clean for meson
Failed to build meson

Pip is missing the 'wheel' package necessary to build a wheel from
sources, if it fails to do that, it falls back to the good old:
$ setup.py install meson
which succeeds and no harm was done. However, seeing an error in the
log always raises eyebrows, so let's fix that very simply by installing
the 'wheel' package which is available on all supported platforms.

Signed-off-by: Erik Skultety 
---
Alternatively, we could use --no-cache-dir with pip install, but I'm not sure
whether it would be enough with new versions of pip. I still feel like
installing the 'wheel' package explicitly is a more transparent and safe fix
even though we don't benefit from the resulting meson wheel package inside
containers at all.

 guests/vars/mappings.yml  | 4 
 guests/vars/projects/base.yml | 1 +
 2 files changed, 5 insertions(+)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 753f0fe..4a19fb4 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -909,6 +909,10 @@ mappings:
 default: python3-setuptools
 FreeBSD: py37-setuptools
 
+  python3-wheel:
+default: python3-wheel
+FreeBSD: py37-wheel
+
   qemu-img:
 default: qemu-utils
 rpm: qemu-img
diff --git a/guests/vars/projects/base.yml b/guests/vars/projects/base.yml
index 29c10b4..81c4462 100644
--- a/guests/vars/projects/base.yml
+++ b/guests/vars/projects/base.yml
@@ -28,6 +28,7 @@ packages:
   - python3
   - python3-pip
   - python3-setuptools
+  - python3-wheel
   - rpmbuild
   - screen
   - strace
-- 
2.25.3

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance

[Daniel P . Berrangé]

On Wed, Apr 29, 2020 at 11:10:41AM +0200, Andrea Bolognani wrote:
> On Tue, 2020-03-31 at 15:28 +0100, Daniel P. Berrangé wrote:
> > Currently when creating a Dockerfile for a container, we include the
> > full set of base packages, along with the packages for the project
> > itself. If building a Perl binding, this would require us to install
> > the base package, libvirt packages and Perl packages. With the use
> > of the "--inherit libvirt-fedora-30" arg, it is possible to have a
> > dockerfile that only adds the Perl packages, getting everything
> > else from the parent image.
> > 
> > For example, a full Dockerfile for libvirt-go would thus be:
> > 
> >   FROM libvirt-centos-7:latest
> > 
> >   RUN yum install -y \
> >   golang && \
> >   yum autoremove -y && \
> >   yum clean all -y
> > 
> > Note there is no need to set ENV either, as that's inherited from the
> > parent container.
> 
> I marked this for review and then kept forgetting to get around to
> it, sorry!
> 
> I like the idea of reusing existing images, as the layering would
> result in significant savings when it comes to disk space and fetch
> times.
> 
> However, as we discussed in the past, there are two possible
> scenarios for subprojects such as libvirt-go:
> 
>   * include dependencies for both libvirt and libvirt-go, then build
> both projects in the resulting container;
> 
>   * include dependencies for libvirt-go along with distro-provided
> packages for libvirt, and only build libvirt-go.
> 
> This would cover the former case, but not the latter one. And, if I
> remember correctly, libvirt-go was one of the projects that would
> benefit more from the latter, because it's supposed to be buildable
> against various versions of libvirt.
> 
> So what I think we need is an additional flag that can be used to
> choose one of the two possible behaviors. This wouldn't be limited
> to the Dockerfile generator, since (unlike inheritance) it can apply
> also to VM management.

I think this problem is tangential to container inheritance and so
doesn't need to be dealt with here.

Instead, it should be solved by simply defining another project
"libvirt-devel", or "libvirt-dist" which pulls in the pre-built
distro packages for libvirt.

> As an additional point, we really need to figure out a good way to
> store dependencies between projects into lcitool itself, so that you
> can tell it that you're interested in building eg. libosinfo and it
> will automatically take care of making osinfo-db-tools and osinfo-db
> available to you, either by installing the binary packages or their
> build dependencies. This is not a strict requirement for container
> inheritance, I think, but the more we go on the more this limitation
> is becoming painful.

I'm not really experiencing this as a painpoint from the container CI
side.

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

[libvirt-ci PATCH] containers: New top-level directory

[Andrea Bolognani]

Let's keep the repository structure tidy.

Signed-off-by: Andrea Bolognani 
---
 .gitlab-ci.yml   | 2 +-
 {check-dco => containers/check-dco}/Dockerfile   | 0
 {check-dco => containers/check-dco}/check-dco.py | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename {check-dco => containers/check-dco}/Dockerfile (100%)
 rename {check-dco => containers/check-dco}/check-dco.py (100%)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 7735bb3..6de69d6 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -12,7 +12,7 @@ stages:
 - docker info
 - docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p 
${CI_REGISTRY_PASSWORD}
   script:
-- docker build --tag ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG} 
${NAME}
+- docker build --tag ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG} 
containers/${NAME}
 - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
   after_script:
 - docker logout
diff --git a/check-dco/Dockerfile b/containers/check-dco/Dockerfile
similarity index 100%
rename from check-dco/Dockerfile
rename to containers/check-dco/Dockerfile
diff --git a/check-dco/check-dco.py b/containers/check-dco/check-dco.py
similarity index 100%
rename from check-dco/check-dco.py
rename to containers/check-dco/check-dco.py
-- 
2.25.4

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance

[Andrea Bolognani]

On Tue, 2020-03-31 at 15:28 +0100, Daniel P. Berrangé wrote:
> Currently when creating a Dockerfile for a container, we include the
> full set of base packages, along with the packages for the project
> itself. If building a Perl binding, this would require us to install
> the base package, libvirt packages and Perl packages. With the use
> of the "--inherit libvirt-fedora-30" arg, it is possible to have a
> dockerfile that only adds the Perl packages, getting everything
> else from the parent image.
> 
> For example, a full Dockerfile for libvirt-go would thus be:
> 
>   FROM libvirt-centos-7:latest
> 
>   RUN yum install -y \
>   golang && \
>   yum autoremove -y && \
>   yum clean all -y
> 
> Note there is no need to set ENV either, as that's inherited from the
> parent container.

I marked this for review and then kept forgetting to get around to
it, sorry!

I like the idea of reusing existing images, as the layering would
result in significant savings when it comes to disk space and fetch
times.

However, as we discussed in the past, there are two possible
scenarios for subprojects such as libvirt-go:

  * include dependencies for both libvirt and libvirt-go, then build
both projects in the resulting container;

  * include dependencies for libvirt-go along with distro-provided
packages for libvirt, and only build libvirt-go.

This would cover the former case, but not the latter one. And, if I
remember correctly, libvirt-go was one of the projects that would
benefit more from the latter, because it's supposed to be buildable
against various versions of libvirt.

So what I think we need is an additional flag that can be used to
choose one of the two possible behaviors. This wouldn't be limited
to the Dockerfile generator, since (unlike inheritance) it can apply
also to VM management.

As an additional point, we really need to figure out a good way to
store dependencies between projects into lcitool itself, so that you
can tell it that you're interested in building eg. libosinfo and it
will automatically take care of making osinfo-db-tools and osinfo-db
available to you, either by installing the binary packages or their
build dependencies. This is not a strict requirement for container
inheritance, I think, but the more we go on the more this limitation
is becoming painful.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] docs: Describe protected virtualization guest setup

[Viktor Mihajlovski]

On 4/28/20 5:58 PM, Boris Fiuczynski wrote:

From: Viktor Mihajlovski 

Protected virtualization/IBM Secure Execution for Linux protects
guest memory and state from the host.

Add some basic information about technology and a brief guide
on setting up secure guests with libvirt.

Signed-off-by: Viktor Mihajlovski 
Reviewed-by: Boris Fiuczynski 

   ^-- Should be changed to s-o-b before applying.

Reviewed-by: Paulo de Rezende Pinatti 
---
  docs/kbase.html.in  |   3 +
  docs/kbase/protected_virtualization.rst | 188 
  2 files changed, 191 insertions(+)
  create mode 100644 docs/kbase/protected_virtualization.rst

[...]
--
Kind Regards,
   Viktor

Re: [libvirt-ci PATCH 0/2] Move MinGW builds back to Fedora Rawhide

[Andrea Bolognani]

On Mon, 2020-04-20 at 12:11 +0200, Andrea Bolognani wrote:
> Andrea Bolognani (2):
>   lcitool: Prepare Fedora Rawhide for MinGW builds
>   Move MinGW builds back to Fedora Rawhide
> 
>  guests/host_vars/libvirt-fedora-30/main.yml  | 10 --
>  guests/host_vars/libvirt-fedora-rawhide/main.yml | 10 ++
>  guests/playbooks/build/jobs/defaults.yml |  2 +-
>  jenkins/jobs/defaults.yaml   |  2 +-
>  4 files changed, 12 insertions(+), 12 deletions(-)

ping

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 2/2] CONTRIBUTING: Include note about build system tools

[Andrea Bolognani]

On Mon, 2020-04-27 at 14:01 +0200, Andrea Bolognani wrote:
> On Mon, 2020-04-27 at 12:19 +0100, Daniel P. Berrangé wrote:
> > Just listing the extra packages is simpler than the group name.
> > 
> >dnf install gcc make automake libtool autoconf rpm-build
> 
> Point taken, the list is short and unlikely to change much.
> 
> Can I have your Reviewed-by for the series if I squash your version
> in?

So, does that sound like a plan? I'd rather not have this very
simple documentation patch linger on the list for too long :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Dr. David Alan Gilbert]

* Yan Zhao (yan.y.z...@intel.com) wrote:
> On Tue, Apr 28, 2020 at 10:14:37PM +0800, Dr. David Alan Gilbert wrote:
> > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert wrote:
> > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert 
> > > > > wrote:
> > > > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > > > On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote:
> > > > > > > > > From: Yan Zhao
> > > > > > > > > Sent: Tuesday, April 21, 2020 10:37 AM
> > > > > > > > > 
> > > > > > > > > On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson 
> > > > > > > > > wrote:
> > > > > > > > > > On Sun, 19 Apr 2020 21:24:57 -0400
> > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > >
> > > > > > > > > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck 
> > > > > > > > > > > wrote:
> > > > > > > > > > > > On Fri, 17 Apr 2020 05:52:02 -0400
> > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia 
> > > > > > > > > > > > > Huck wrote:
> > > > > > > > > > > > > > On Mon, 13 Apr 2020 01:52:01 -0400
> > > > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > This patchset introduces a migration_version 
> > > > > > > > > > > > > > > attribute under sysfs
> > > > > > > > > of VFIO
> > > > > > > > > > > > > > > Mediated devices.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > This migration_version attribute is used to check 
> > > > > > > > > > > > > > > migration
> > > > > > > > > compatibility
> > > > > > > > > > > > > > > between two mdev devices.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Currently, it has two locations:
> > > > > > > > > > > > > > > (1) under mdev_type node,
> > > > > > > > > > > > > > > which can be used even before device 
> > > > > > > > > > > > > > > creation, but only for
> > > > > > > > > mdev
> > > > > > > > > > > > > > > devices of the same mdev type.
> > > > > > > > > > > > > > > (2) under mdev device node,
> > > > > > > > > > > > > > > which can only be used after the mdev devices 
> > > > > > > > > > > > > > > are created, but
> > > > > > > > > the src
> > > > > > > > > > > > > > > and target mdev devices are not necessarily 
> > > > > > > > > > > > > > > be of the same
> > > > > > > > > mdev type
> > > > > > > > > > > > > > > (The second location is newly added in v5, in 
> > > > > > > > > > > > > > > order to keep
> > > > > > > > > consistent
> > > > > > > > > > > > > > > with the migration_version node for migratable 
> > > > > > > > > > > > > > > pass-though
> > > > > > > > > devices)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > What is the relationship between those two 
> > > > > > > > > > > > > > attributes?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > (1) is for mdev devices specifically, and (2) is 
> > > > > > > > > > > > > provided to keep the
> > > > > > > > > same
> > > > > > > > > > > > > sysfs interface as with non-mdev cases. so (2) is for 
> > > > > > > > > > > > > both mdev
> > > > > > > > > devices and
> > > > > > > > > > > > > non-mdev devices.
> > > > > > > > > > > > >
> > > > > > > > > > > > > in future, if we enable vfio-pci vendor ops, (i.e. a 
> > > > > > > > > > > > > non-mdev device
> > > > > > > > > > > > > is binding to vfio-pci, but is able to register 
> > > > > > > > > > > > > migration region and do
> > > > > > > > > > > > > migration transactions from a vendor provided 
> > > > > > > > > > > > > affiliate driver),
> > > > > > > > > > > > > the vendor driver would export (2) directly, under 
> > > > > > > > > > > > > device node.
> > > > > > > > > > > > > It is not able to provide (1) as there're no mdev 
> > > > > > > > > > > > > devices involved.
> > > > > > > > > > > >
> > > > > > > > > > > > Ok, creating an alternate attribute for non-mdev 
> > > > > > > > > > > > devices makes sense.
> > > > > > > > > > > > However, wouldn't that rather be a case (3)? The change 
> > > > > > > > > > > > here only
> > > > > > > > > > > > refers to mdev devices.
> > > > > > > > > > > >
> > > > > > > > > > > as you pointed below, (3) and (2) serve the same purpose.
> > > > > > > > > > > and I think a possible usage is to migrate between a 
> > > > > > > > > > > non-mdev device and
> > > > > > > > > > > an mdev device. so I think it's better for them both to 
> > > > > > > > > > > use (2) rather
> > > > > > > > > > > than creating (3).
> > > > > > > > > >
> > > > > > > > > > An mdev type is meant to define a software compatible 
> > > > > > > > > > interface, so in
> > > > > > > > > > the case of mdev->mdev migration, doesn't migrating to a 
> > > > > > > > > > different type
> > > > > > > > > > fail the most basic of compatibility tests that we expect 
> > > > > > > > > > userspace to
> > > > > > > > > > perform?  IOW, if two md

Re: [libvirt-ci PATCH 3/4] lcitool: Store packaging information as a dictionary

[Erik Skultety]

On Tue, Apr 28, 2020 at 03:37:59PM +0200, Andrea Bolognani wrote:
> Ansible and Python both support actual dictionaries, so make use
> of them in the inventory instead of emulating them by using
> variable names sharing the same prefix.
> 
> Signed-off-by: Andrea Bolognani 
Reviewed-by: Erik Skultety 

Re: [libvirt-ci PATCH 4/4] lcitool: Store paths information as a dictionary

[Erik Skultety]

On Tue, Apr 28, 2020 at 03:38:00PM +0200, Andrea Bolognani wrote:
> Ansible and Python both support actual dictionaries, so make use
> of them in the inventory instead of having a bunch of randomly
> named variables lumped together.
> 
> This commit is best viewed with 'git show -w'.
> 
> Signed-off-by: Andrea Bolognani 
Reviewed-by: Erik Skultety 

Re: [libvirt-ci PATCH 2/4] lcitool: Store OS information a dictionary

[Erik Skultety]

On Tue, Apr 28, 2020 at 03:37:58PM +0200, Andrea Bolognani wrote:
> Ansible and Python both support actual dictionaries, so make use
> of them in the inventory instead of emulating them by using
> variable names sharing the same prefix.
> 
> Signed-off-by: Andrea Bolognani 
Reviewed-by: Erik Skultety 

Re: [PATCH V3 5/5] cpu: Introduce getHost support for ARM CPU driver

[Zhenyu Zheng]

Hi Jiri，

Thanks alot for the help, I've updated the series to v4 and also attached
pipeline results for each patch as suggested.

BR,

On Tue, Apr 28, 2020 at 7:11 PM Zhenyu Zheng 
wrote:

> Thanks alot I will check again about the patch series.
>
> On Tue, Apr 28, 2020 at 6:39 PM Jiri Denemark  wrote:
>
>> On Wed, Apr 22, 2020 at 15:14:01 +0800, Zhenyu Zheng wrote:
>> >  gitlab CI testing as suggested:
>> > https://gitlab.com/ZhengZhenyu/libvirt/pipelines/134657317
>>
>> Sending results of CI pipeline makes sense only when the code submitted
>> for CI is exactly the same as submitted for review. You should just push
>> the branch with your local changes to your gitlab repo rather than
>> somehow combining all patches you sent into one or even make additional
>> changes.
>>
>> Apparently the patch pushed to gitlab is a bit different than this
>> series as this series cannot be compiled due to a missing "return NULL;"
>> line in patch 4/5.
>>
>> Anyway, I'm trying to get some ARM hosts and hoping to find at least one
>> where your series would actually detect the host CPU to see how this
>> works in real life and to confirm (or not) my suspicions. No need to
>> resent this series to fix the compilation error yet, I'll fix it myself
>> for testing.
>>
>> Thank you for your patience.
>>
>> Jirka
>>
>

Re: [PATCH V4 4/5] cpu: Introduce getHost support for ARM CPU driver

[Zhenyu Zheng]

pipeline test result for this patch:
https://gitlab.com/ZhengZhenyu/libvirt/pipelines/140926826

On Wed, Apr 29, 2020 at 3:55 PM Zhenyu Zheng 
wrote:

> Introduce getHost support for ARM CPU driver, read
> CPU vendor_id, part_id and flags from registers
> directly. These codes will only be compiled on
> aarch64 hardwares.
>
> Signed-off-by: Zhenyu Zheng 
> ---
>  src/cpu/cpu_arm.c | 201 ++
>  1 file changed, 201 insertions(+)
>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index 24404eac2c..887932cc5a 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -21,6 +21,10 @@
>   */
>
>  #include 
> +#if defined(__aarch64__)
> +# include 
> +# include 
> +#endif
>
>  #include "viralloc.h"
>  #include "cpu.h"
> @@ -31,6 +35,12 @@
>  #include "virxml.h"
>
>  #define VIR_FROM_THIS VIR_FROM_CPU
> +#if defined(__aarch64__)
> +/* Shift bit mask for parsing cpu flags */
> +# define BIT_SHIFTS(n) (1UL << (n))
> +/* The current max number of cpu flags on ARM is 32 */
> +# define MAX_CPU_FLAGS 32
> +#endif
>
>  VIR_LOG_INIT("cpu.cpu_arm");
>
> @@ -495,12 +505,203 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)
>  return 0;
>  }
>
> +#if defined(__aarch64__)
> +/**
> + * virCPUarmCpuDataFromRegs:
> + *
> + * @data: 64-bit arm CPU specific data
> + *
> + * Fetches CPU vendor_id and part_id from MIDR_EL1 register, parse CPU
> + * flags from AT_HWCAP. There are currently 32 valid flags  on ARM arch
> + * represented by each bit.
> + */
> +static int
> +virCPUarmCpuDataFromRegs(virCPUarmData *data)
> +{
> +/* Generate human readable flag list according to the order of */
> +/* AT_HWCAP bit map */
> +const char *flag_list[MAX_CPU_FLAGS] = {
> +"fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2",
> +"crc32", "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm",
> +"jscvt", "fcma", "lrcpc", "dcpop", "sha3", "sm3", "sm4",
> +"asimddp", "sha512", "sve", "asimdfhm", "dit", "uscat",
> +"ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"};
> +unsigned long cpuid, hwcaps;
> +char **features = NULL;
> +g_autofree char *cpu_feature_str = NULL;
> +int cpu_feature_index = 0;
> +size_t i;
> +
> +if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("CPUID registers unavailable"));
> +return -1;
> +}
> +
> +/* read the cpuid data from MIDR_EL1 register */
> +asm("mrs %0, MIDR_EL1" : "=r" (cpuid));
> +VIR_DEBUG("CPUID read from register:  0x%016lx", cpuid);
> +
> +/* parse the coresponding part_id bits */
> +data->pvr = cpuid>>4&0xFFF;
> +/* parse the coresponding vendor_id bits */
> +data->vendor_id = cpuid>>24&0xFF;
> +
> +hwcaps = getauxval(AT_HWCAP);
> +VIR_DEBUG("CPU flags read from register:  0x%016lx", hwcaps);
> +
> +if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0)
> +return -1;
> +
> +/* shift bit map mask to parse for CPU flags */
> +for (i = 0; i< MAX_CPU_FLAGS; i++) {
> +if (hwcaps & BIT_SHIFTS(i)) {
> +features[cpu_feature_index] = g_strdup(flag_list[i]);
> +cpu_feature_index++;
> +}
> +}
> +
> +if (cpu_feature_index > 1) {
> +cpu_feature_str = virStringListJoin((const char **)features, " ");
> +if (!cpu_feature_str)
> +goto error;
> +}
> +data->features = g_strdup(cpu_feature_str);
> +
> +return 0;
> +
> + error:
> +virStringListFree(features);
> +return -1;
> +}
> +
> +static int
> +virCPUarmDataParseFeatures(virCPUDefPtr cpu,
> +   const virCPUarmData *cpuData)
> +{
> +int ret = -1;
> +size_t i;
> +char **features;
> +
> +if (!cpu || !cpuData)
> +return ret;
> +
> +if (!(features = virStringSplitCount(cpuData->features, " ",
> + 0, &cpu->nfeatures)))
> +return ret;
> +if (cpu->nfeatures) {
> +if (VIR_ALLOC_N(cpu->features, cpu->nfeatures) < 0)
> +goto error;
> +
> +for (i = 0; i < cpu->nfeatures; i++) {
> +cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE;
> +cpu->features[i].name = g_strdup(features[i]);
> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +virStringListFree(features);
> +return ret;
> +
> + error:
> +for (i = 0; i < cpu->nfeatures; i++)
> +VIR_FREE(cpu->features[i].name);
> +VIR_FREE(cpu->features);
> +cpu->nfeatures = 0;
> +goto cleanup;
> +}
> +
> +static int
> +virCPUarmDecode(virCPUDefPtr cpu,
> +const virCPUarmData *cpuData,
> +virDomainCapsCPUModelsPtr models)
> +{
> +virCPUarmMapPtr map;
> +virCPUarmModelPtr model;
> +virCPUarmVendorPtr vendor = NULL;
> +
> +if (!cpuData || !(map = virCPUarmGetMap()))
> +return -1;
> +
> +if (!(model = virCPUarmModelFindByPVR(map, cpuData->pvr))

Re: [PATCH V4 5/5] cpu_map: Introduce ARM cpu models

[Zhenyu Zheng]

pipeline test result for this patch:
https://gitlab.com/ZhengZhenyu/libvirt/-/commit/89878b01d1ed6848fe9dfb46192f8f5694efdd32


On Wed, Apr 29, 2020 at 3:56 PM Zhenyu Zheng 
wrote:

> Introduce vendors and some commonly used models
> for ARM arch, these will be used for virConnectionGetCapabilities
> for ARM CPUs.
>
> Signed-off-by: Zhenyu Zheng 
> ---
>  src/cpu_map/Makefile.inc.am   |  7 +++
>  src/cpu_map/arm_Falkor.xml| 16 
>  src/cpu_map/arm_Kunpeng-920.xml   | 24 
>  src/cpu_map/arm_ThunderX299xx.xml | 16 
>  src/cpu_map/arm_cortex-a53.xml| 16 
>  src/cpu_map/arm_cortex-a57.xml| 15 +++
>  src/cpu_map/arm_cortex-a72.xml| 15 +++
>  src/cpu_map/arm_vendors.xml   | 14 ++
>  src/cpu_map/index.xml | 15 +++
>  9 files changed, 138 insertions(+)
>  create mode 100644 src/cpu_map/arm_Falkor.xml
>  create mode 100644 src/cpu_map/arm_Kunpeng-920.xml
>  create mode 100644 src/cpu_map/arm_ThunderX299xx.xml
>  create mode 100644 src/cpu_map/arm_cortex-a53.xml
>  create mode 100644 src/cpu_map/arm_cortex-a57.xml
>  create mode 100644 src/cpu_map/arm_cortex-a72.xml
>  create mode 100644 src/cpu_map/arm_vendors.xml
>
> diff --git a/src/cpu_map/Makefile.inc.am b/src/cpu_map/Makefile.inc.am
> index be64c9a0d4..5d9190e27d 100644
> --- a/src/cpu_map/Makefile.inc.am
> +++ b/src/cpu_map/Makefile.inc.am
> @@ -2,7 +2,14 @@
>
>  cpumapdir = $(pkgdatadir)/cpu_map
>  cpumap_DATA = \
> + cpu_map/arm_cortex-a53.xml \
> + cpu_map/arm_cortex-a57.xml \
> + cpu_map/arm_cortex-a72.xml \
>   cpu_map/arm_features.xml \
> + cpu_map/arm_Kunpeng-920.xml \
> + cpu_map/arm_ThunderX299xx.xml \
> + cpu_map/arm_Falkor.xml \
> + cpu_map/arm_vendors.xml \
>   cpu_map/index.xml \
>   cpu_map/ppc64_vendors.xml \
>   cpu_map/ppc64_POWER7.xml \
> diff --git a/src/cpu_map/arm_Falkor.xml b/src/cpu_map/arm_Falkor.xml
> new file mode 100644
> index 00..902ed2b6ba
> --- /dev/null
> +++ b/src/cpu_map/arm_Falkor.xml
> @@ -0,0 +1,16 @@
> +
> +  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +  
> +
> diff --git a/src/cpu_map/arm_Kunpeng-920.xml
> b/src/cpu_map/arm_Kunpeng-920.xml
> new file mode 100644
> index 00..668b8b50dc
> --- /dev/null
> +++ b/src/cpu_map/arm_Kunpeng-920.xml
> @@ -0,0 +1,24 @@
> +
> +  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +  
> +
> diff --git a/src/cpu_map/arm_ThunderX299xx.xml
> b/src/cpu_map/arm_ThunderX299xx.xml
> new file mode 100644
> index 00..9ab3d939e9
> --- /dev/null
> +++ b/src/cpu_map/arm_ThunderX299xx.xml
> @@ -0,0 +1,16 @@
> +
> +  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +  
> +
> diff --git a/src/cpu_map/arm_cortex-a53.xml
> b/src/cpu_map/arm_cortex-a53.xml
> new file mode 100644
> index 00..963d6d36e3
> --- /dev/null
> +++ b/src/cpu_map/arm_cortex-a53.xml
> @@ -0,0 +1,16 @@
> +
> +  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +  
> +
> diff --git a/src/cpu_map/arm_cortex-a57.xml
> b/src/cpu_map/arm_cortex-a57.xml
> new file mode 100644
> index 00..92a044ea92
> --- /dev/null
> +++ b/src/cpu_map/arm_cortex-a57.xml
> @@ -0,0 +1,15 @@
> +
> +  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +  
> +
> diff --git a/src/cpu_map/arm_cortex-a72.xml
> b/src/cpu_map/arm_cortex-a72.xml
> new file mode 100644
> index 00..9664eacd7b
> --- /dev/null
> +++ b/src/cpu_map/arm_cortex-a72.xml
> @@ -0,0 +1,15 @@
> +
> +  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +  
> +
> diff --git a/src/cpu_map/arm_vendors.xml b/src/cpu_map/arm_vendors.xml
> new file mode 100644
> index 00..703c2112b1
> --- /dev/null
> +++ b/src/cpu_map/arm_vendors.xml
> @@ -0,0 +1,14 @@
> +
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +
> diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
> index 50b030de29..20646a031c 100644
> --- a/src/cpu_map/index.xml
> +++ b/src/cpu_map/index.xml
> @@ -85,6 +85,21 @@
>
>
>
> +
>  
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
>
>  
> --
> 2.26.2
>

Re: [PATCH V4 3/5] cpu: Add helper funtions to parse vendor and model

[Zhenyu Zheng]

pipeline test result for this patch:
https://gitlab.com/ZhengZhenyu/libvirt/pipelines/140916109

On Wed, Apr 29, 2020 at 3:54 PM Zhenyu Zheng 
wrote:

> Add helper functions to parse vendor and model from
> xml for ARM arch, and use them as callbacks when
> load cpu maps.
>
> Signed-off-by: Zhenyu Zheng 
> ---
>  src/cpu/cpu_arm.c | 171 +-
>  1 file changed, 170 insertions(+), 1 deletion(-)
>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index d8f571cae3..24404eac2c 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -206,6 +206,174 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt
> G_GNUC_UNUSED,
>  return 0;
>  }
>
> +static virCPUarmVendorPtr
> +virCPUarmVendorFindByID(virCPUarmMapPtr map,
> +unsigned long vendor_id)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nvendors; i++) {
> +if (map->vendors[i]->value == vendor_id)
> +return map->vendors[i];
> +}
> +
> +return NULL;
> +}
> +
> +
> +static virCPUarmVendorPtr
> +virCPUarmVendorFindByName(virCPUarmMapPtr map,
> +  const char *name)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nvendors; i++) {
> +if (STREQ(map->vendors[i]->name, name))
> +return map->vendors[i];
> +}
> +
> +return NULL;
> +}
> +
> +
> +static int
> +virCPUarmVendorParse(xmlXPathContextPtr ctxt,
> +   const char *name,
> +   void *data)
> +{
> +virCPUarmMapPtr map = data;
> +virCPUarmVendorPtr vendor = NULL;
> +int ret = -1;
> +
> +if (VIR_ALLOC(vendor) < 0)
> +return ret;
> +
> +vendor->name = g_strdup(name);
> +
> +if (virCPUarmVendorFindByName(map, vendor->name)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("CPU vendor %s already defined"),
> +   vendor->name);
> +goto cleanup;
> +}
> +
> +if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Missing CPU vendor value"));
> +goto cleanup;
> +}
> +
> +if (virCPUarmVendorFindByID(map, vendor->value)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("CPU vendor value 0x%2lx already defined"),
> +   vendor->value);
> +goto cleanup;
> +}
> +
> +if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0)
> +goto cleanup;
> +
> +ret = 0;
> +
> + cleanup:
> +virCPUarmVendorFree(vendor);
> +return ret;
> +
> +}
> +
> +static virCPUarmModelPtr
> +virCPUarmModelFind(virCPUarmMapPtr map,
> +   const char *name)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nmodels; i++) {
> +if (STREQ(map->models[i]->name, name))
> +return map->models[i];
> +}
> +
> +return NULL;
> +}
> +
> +#if defined(__aarch64__)
> +static virCPUarmModelPtr
> +virCPUarmModelFindByPVR(virCPUarmMapPtr map,
> +unsigned long pvr)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nmodels; i++) {
> +if (map->models[i]->data.pvr == pvr)
> +return map->models[i];
> +}
> +
> +return NULL;
> +}
> +#endif
> +
> +static int
> +virCPUarmModelParse(xmlXPathContextPtr ctxt,
> +  const char *name,
> +  void *data)
> +{
> +virCPUarmMapPtr map = data;
> +virCPUarmModel *model;
> +g_autofree xmlNodePtr *nodes = NULL;
> +g_autofree char *vendor = NULL;
> +
> +if (VIR_ALLOC(model) < 0)
> +goto error;
> +
> +model->name = g_strdup(name);
> +
> +if (virCPUarmModelFind(map, model->name)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("CPU model %s already defined"),
> +   model->name);
> +goto error;
> +}
> +
> +if (virXPathBoolean("boolean(./vendor)", ctxt)) {
> +vendor = virXPathString("string(./vendor/@name)", ctxt);
> +if (!vendor) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Invalid vendor element in CPU model %s"),
> +   model->name);
> +goto error;
> +}
> +
> +if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unknown vendor %s referenced by CPU model
> %s"),
> +   vendor, model->name);
> +goto error;
> +}
> +}
> +
> +if (!virXPathBoolean("boolean(./pvr)", ctxt)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Missing PVR information for CPU model %s"),
> +   model->name);
> +goto error;
> +}
> +
> +if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr)
> < 0) {
> +virReportError(VIR_ERR_INTER

Re: [PATCH V4 2/5] cpu: Introduce ARM related structs

[Zhenyu Zheng]

pipeline test results for this patch:
https://gitlab.com/ZhengZhenyu/libvirt/pipelines/140890614

On Wed, Apr 29, 2020 at 3:51 PM Zhenyu Zheng 
wrote:

> Introduce vendor and model struct and related
> cleanup functions for ARM cpu.
>
> Signed-off-by: Zhenyu Zheng 
> ---
>  src/cpu/cpu_arm.c | 75 +++
>  1 file changed, 75 insertions(+)
>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index ee5802198f..d8f571cae3 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -1,6 +1,7 @@
>  /*
>   * cpu_arm.c: CPU driver for arm CPUs
>   *
> + * Copyright (C) 2020 Huawei Technologies Co., Ltd.
>   * Copyright (C) 2013 Red Hat, Inc.
>   * Copyright (C) Canonical Ltd. 2012
>   *
> @@ -23,12 +24,16 @@
>
>  #include "viralloc.h"
>  #include "cpu.h"
> +#include "cpu_arm.h"
>  #include "cpu_map.h"
> +#include "virlog.h"
>  #include "virstring.h"
>  #include "virxml.h"
>
>  #define VIR_FROM_THIS VIR_FROM_CPU
>
> +VIR_LOG_INIT("cpu.cpu_arm");
> +
>  static const virArch archs[] = {
>  VIR_ARCH_ARMV6L,
>  VIR_ARCH_ARMV7B,
> @@ -36,6 +41,21 @@ static const virArch archs[] = {
>  VIR_ARCH_AARCH64,
>  };
>
> +typedef struct _virCPUarmVendor virCPUarmVendor;
> +typedef virCPUarmVendor *virCPUarmVendorPtr;
> +struct _virCPUarmVendor {
> +char *name;
> +unsigned long value;
> +};
> +
> +typedef struct _virCPUarmModel virCPUarmModel;
> +typedef virCPUarmModel *virCPUarmModelPtr;
> +struct _virCPUarmModel {
> +char *name;
> +virCPUarmVendorPtr vendor;
> +virCPUarmData data;
> +};
> +
>  typedef struct _virCPUarmFeature virCPUarmFeature;
>  typedef virCPUarmFeature *virCPUarmFeaturePtr;
>  struct _virCPUarmFeature {
> @@ -64,6 +84,10 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUarmFeature,
> virCPUarmFeatureFree);
>  typedef struct _virCPUarmMap virCPUarmMap;
>  typedef virCPUarmMap *virCPUarmMapPtr;
>  struct _virCPUarmMap {
> +size_t nvendors;
> +virCPUarmVendorPtr *vendors;
> +size_t nmodels;
> +virCPUarmModelPtr *models;
>  GPtrArray *features;
>  };
>
> @@ -81,12 +105,62 @@ virCPUarmMapNew(void)
>  return map;
>  }
>
> +static void
> +virCPUarmDataClear(virCPUarmData *data)
> +{
> +if (!data)
> +return;
> +
> +VIR_FREE(data->features);
> +}
> +
> +static void
> +virCPUarmDataFree(virCPUDataPtr cpuData)
> +{
> +if (!cpuData)
> +return;
> +
> +virCPUarmDataClear(&cpuData->data.arm);
> +VIR_FREE(cpuData);
> +}
> +
> +static void
> +virCPUarmModelFree(virCPUarmModelPtr model)
> +{
> +if (!model)
> +return;
> +
> +virCPUarmDataClear(&model->data);
> +g_free(model->name);
> +g_free(model);
> +}
> +
> +static void
> +virCPUarmVendorFree(virCPUarmVendorPtr vendor)
> +{
> +if (!vendor)
> +return;
> +
> +g_free(vendor->name);
> +g_free(vendor);
> +}
> +
>  static void
>  virCPUarmMapFree(virCPUarmMapPtr map)
>  {
>  if (!map)
>  return;
>
> +size_t i;
> +
> +for (i = 0; i < map->nmodels; i++)
> +virCPUarmModelFree(map->models[i]);
> +g_free(map->models);
> +
> +for (i = 0; i < map->nvendors; i++)
> +virCPUarmVendorFree(map->vendors[i]);
> +g_free(map->vendors);
> +
>  g_ptr_array_free(map->features, TRUE);
>
>  g_free(map);
> @@ -259,6 +333,7 @@ struct cpuArchDriver cpuDriverArm = {
>  .compare = virCPUarmCompare,
>  .decode = NULL,
>  .encode = NULL,
> +.dataFree = virCPUarmDataFree,
>  .baseline = virCPUarmBaseline,
>  .update = virCPUarmUpdate,
>  .validateFeatures = virCPUarmValidateFeatures,
> --
> 2.26.2
>

Re: [PATCH V4 1/5] cpu: Introduce virCPUarmData to virCPUData

[Zhenyu Zheng]

pipeline test result for this patch:
https://gitlab.com/ZhengZhenyu/libvirt/pipelines/140885336

On Wed, Apr 29, 2020 at 3:57 PM Zhenyu Zheng 
wrote:

> Introduce virCPUarmData to virCPUData
>
> Signed-off-by: Zhenyu Zheng 
> ---
>  src/cpu/Makefile.inc.am |  1 +
>  src/cpu/cpu.h   |  2 ++
>  src/cpu/cpu_arm_data.h  | 31 +++
>  3 files changed, 34 insertions(+)
>  create mode 100644 src/cpu/cpu_arm_data.h
>
> diff --git a/src/cpu/Makefile.inc.am b/src/cpu/Makefile.inc.am
> index 0abeee87b6..228112a3c6 100644
> --- a/src/cpu/Makefile.inc.am
> +++ b/src/cpu/Makefile.inc.am
> @@ -9,6 +9,7 @@ CPU_SOURCES = \
>   cpu/cpu_s390.h \
>   cpu/cpu_s390.c \
>   cpu/cpu_arm.h \
> + cpu/cpu_arm_data.h \
>   cpu/cpu_arm.c \
>   cpu/cpu_ppc64.h \
>   cpu/cpu_ppc64.c \
> diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
> index f779d2be17..ec22a183a1 100644
> --- a/src/cpu/cpu.h
> +++ b/src/cpu/cpu.h
> @@ -27,6 +27,7 @@
>  #include "cpu_conf.h"
>  #include "cpu_x86_data.h"
>  #include "cpu_ppc64_data.h"
> +#include "cpu_arm_data.h"
>
>
>  typedef struct _virCPUData virCPUData;
> @@ -36,6 +37,7 @@ struct _virCPUData {
>  union {
>  virCPUx86Data x86;
>  virCPUppc64Data ppc64;
> +virCPUarmData arm;
>  /* generic driver needs no data */
>  } data;
>  };
> diff --git a/src/cpu/cpu_arm_data.h b/src/cpu/cpu_arm_data.h
> new file mode 100644
> index 00..cf12ca8c2e
> --- /dev/null
> +++ b/src/cpu/cpu_arm_data.h
> @@ -0,0 +1,31 @@
> +/*
> + * cpu_arm_data.h: 64-bit arm CPU specific data
> + *
> + * Copyright (C) 2020 Huawei Technologies Co., Ltd.
> + *
> + * 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
> + * .
> + *
> + */
> +
> +#pragma once
> +
> +#define VIR_CPU_ARM_DATA_INIT { 0 }
> +
> +typedef struct _virCPUarmData virCPUarmData;
> +struct _virCPUarmData {
> +unsigned long vendor_id;
> +unsigned long pvr;
> +char *features;
> +};
> --
> 2.26.2
>

[PATCH V4 1/5] cpu: Introduce virCPUarmData to virCPUData

[Zhenyu Zheng]

Introduce virCPUarmData to virCPUData

Signed-off-by: Zhenyu Zheng 
---
 src/cpu/Makefile.inc.am |  1 +
 src/cpu/cpu.h   |  2 ++
 src/cpu/cpu_arm_data.h  | 31 +++
 3 files changed, 34 insertions(+)
 create mode 100644 src/cpu/cpu_arm_data.h

diff --git a/src/cpu/Makefile.inc.am b/src/cpu/Makefile.inc.am
index 0abeee87b6..228112a3c6 100644
--- a/src/cpu/Makefile.inc.am
+++ b/src/cpu/Makefile.inc.am
@@ -9,6 +9,7 @@ CPU_SOURCES = \
  cpu/cpu_s390.h \
  cpu/cpu_s390.c \
  cpu/cpu_arm.h \
+ cpu/cpu_arm_data.h \
  cpu/cpu_arm.c \
  cpu/cpu_ppc64.h \
  cpu/cpu_ppc64.c \
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index f779d2be17..ec22a183a1 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -27,6 +27,7 @@
 #include "cpu_conf.h"
 #include "cpu_x86_data.h"
 #include "cpu_ppc64_data.h"
+#include "cpu_arm_data.h"


 typedef struct _virCPUData virCPUData;
@@ -36,6 +37,7 @@ struct _virCPUData {
 union {
 virCPUx86Data x86;
 virCPUppc64Data ppc64;
+virCPUarmData arm;
 /* generic driver needs no data */
 } data;
 };
diff --git a/src/cpu/cpu_arm_data.h b/src/cpu/cpu_arm_data.h
new file mode 100644
index 00..cf12ca8c2e
--- /dev/null
+++ b/src/cpu/cpu_arm_data.h
@@ -0,0 +1,31 @@
+/*
+ * cpu_arm_data.h: 64-bit arm CPU specific data
+ *
+ * Copyright (C) 2020 Huawei Technologies Co., Ltd.
+ *
+ * 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
+ * .
+ *
+ */
+
+#pragma once
+
+#define VIR_CPU_ARM_DATA_INIT { 0 }
+
+typedef struct _virCPUarmData virCPUarmData;
+struct _virCPUarmData {
+unsigned long vendor_id;
+unsigned long pvr;
+char *features;
+};
-- 
2.26.2

[PATCH V4 5/5] cpu_map: Introduce ARM cpu models

[Zhenyu Zheng]

Introduce vendors and some commonly used models
for ARM arch, these will be used for virConnectionGetCapabilities
for ARM CPUs.

Signed-off-by: Zhenyu Zheng 
---
 src/cpu_map/Makefile.inc.am   |  7 +++
 src/cpu_map/arm_Falkor.xml| 16 
 src/cpu_map/arm_Kunpeng-920.xml   | 24 
 src/cpu_map/arm_ThunderX299xx.xml | 16 
 src/cpu_map/arm_cortex-a53.xml| 16 
 src/cpu_map/arm_cortex-a57.xml| 15 +++
 src/cpu_map/arm_cortex-a72.xml| 15 +++
 src/cpu_map/arm_vendors.xml   | 14 ++
 src/cpu_map/index.xml | 15 +++
 9 files changed, 138 insertions(+)
 create mode 100644 src/cpu_map/arm_Falkor.xml
 create mode 100644 src/cpu_map/arm_Kunpeng-920.xml
 create mode 100644 src/cpu_map/arm_ThunderX299xx.xml
 create mode 100644 src/cpu_map/arm_cortex-a53.xml
 create mode 100644 src/cpu_map/arm_cortex-a57.xml
 create mode 100644 src/cpu_map/arm_cortex-a72.xml
 create mode 100644 src/cpu_map/arm_vendors.xml

diff --git a/src/cpu_map/Makefile.inc.am b/src/cpu_map/Makefile.inc.am
index be64c9a0d4..5d9190e27d 100644
--- a/src/cpu_map/Makefile.inc.am
+++ b/src/cpu_map/Makefile.inc.am
@@ -2,7 +2,14 @@

 cpumapdir = $(pkgdatadir)/cpu_map
 cpumap_DATA = \
+ cpu_map/arm_cortex-a53.xml \
+ cpu_map/arm_cortex-a57.xml \
+ cpu_map/arm_cortex-a72.xml \
  cpu_map/arm_features.xml \
+ cpu_map/arm_Kunpeng-920.xml \
+ cpu_map/arm_ThunderX299xx.xml \
+ cpu_map/arm_Falkor.xml \
+ cpu_map/arm_vendors.xml \
  cpu_map/index.xml \
  cpu_map/ppc64_vendors.xml \
  cpu_map/ppc64_POWER7.xml \
diff --git a/src/cpu_map/arm_Falkor.xml b/src/cpu_map/arm_Falkor.xml
new file mode 100644
index 00..902ed2b6ba
--- /dev/null
+++ b/src/cpu_map/arm_Falkor.xml
@@ -0,0 +1,16 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/src/cpu_map/arm_Kunpeng-920.xml
b/src/cpu_map/arm_Kunpeng-920.xml
new file mode 100644
index 00..668b8b50dc
--- /dev/null
+++ b/src/cpu_map/arm_Kunpeng-920.xml
@@ -0,0 +1,24 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/src/cpu_map/arm_ThunderX299xx.xml
b/src/cpu_map/arm_ThunderX299xx.xml
new file mode 100644
index 00..9ab3d939e9
--- /dev/null
+++ b/src/cpu_map/arm_ThunderX299xx.xml
@@ -0,0 +1,16 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/src/cpu_map/arm_cortex-a53.xml b/src/cpu_map/arm_cortex-a53.xml
new file mode 100644
index 00..963d6d36e3
--- /dev/null
+++ b/src/cpu_map/arm_cortex-a53.xml
@@ -0,0 +1,16 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/src/cpu_map/arm_cortex-a57.xml b/src/cpu_map/arm_cortex-a57.xml
new file mode 100644
index 00..92a044ea92
--- /dev/null
+++ b/src/cpu_map/arm_cortex-a57.xml
@@ -0,0 +1,15 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/src/cpu_map/arm_cortex-a72.xml b/src/cpu_map/arm_cortex-a72.xml
new file mode 100644
index 00..9664eacd7b
--- /dev/null
+++ b/src/cpu_map/arm_cortex-a72.xml
@@ -0,0 +1,15 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+  
+
diff --git a/src/cpu_map/arm_vendors.xml b/src/cpu_map/arm_vendors.xml
new file mode 100644
index 00..703c2112b1
--- /dev/null
+++ b/src/cpu_map/arm_vendors.xml
@@ -0,0 +1,14 @@
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
index 50b030de29..20646a031c 100644
--- a/src/cpu_map/index.xml
+++ b/src/cpu_map/index.xml
@@ -85,6 +85,21 @@
   

   
+
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
   
 
-- 
2.26.2

[PATCH V4 4/5] cpu: Introduce getHost support for ARM CPU driver

[Zhenyu Zheng]

Introduce getHost support for ARM CPU driver, read
CPU vendor_id, part_id and flags from registers
directly. These codes will only be compiled on
aarch64 hardwares.

Signed-off-by: Zhenyu Zheng 
---
 src/cpu/cpu_arm.c | 201 ++
 1 file changed, 201 insertions(+)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 24404eac2c..887932cc5a 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -21,6 +21,10 @@
  */

 #include 
+#if defined(__aarch64__)
+# include 
+# include 
+#endif

 #include "viralloc.h"
 #include "cpu.h"
@@ -31,6 +35,12 @@
 #include "virxml.h"

 #define VIR_FROM_THIS VIR_FROM_CPU
+#if defined(__aarch64__)
+/* Shift bit mask for parsing cpu flags */
+# define BIT_SHIFTS(n) (1UL << (n))
+/* The current max number of cpu flags on ARM is 32 */
+# define MAX_CPU_FLAGS 32
+#endif

 VIR_LOG_INIT("cpu.cpu_arm");

@@ -495,12 +505,203 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)
 return 0;
 }

+#if defined(__aarch64__)
+/**
+ * virCPUarmCpuDataFromRegs:
+ *
+ * @data: 64-bit arm CPU specific data
+ *
+ * Fetches CPU vendor_id and part_id from MIDR_EL1 register, parse CPU
+ * flags from AT_HWCAP. There are currently 32 valid flags  on ARM arch
+ * represented by each bit.
+ */
+static int
+virCPUarmCpuDataFromRegs(virCPUarmData *data)
+{
+/* Generate human readable flag list according to the order of */
+/* AT_HWCAP bit map */
+const char *flag_list[MAX_CPU_FLAGS] = {
+"fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2",
+"crc32", "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm",
+"jscvt", "fcma", "lrcpc", "dcpop", "sha3", "sm3", "sm4",
+"asimddp", "sha512", "sve", "asimdfhm", "dit", "uscat",
+"ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"};
+unsigned long cpuid, hwcaps;
+char **features = NULL;
+g_autofree char *cpu_feature_str = NULL;
+int cpu_feature_index = 0;
+size_t i;
+
+if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("CPUID registers unavailable"));
+return -1;
+}
+
+/* read the cpuid data from MIDR_EL1 register */
+asm("mrs %0, MIDR_EL1" : "=r" (cpuid));
+VIR_DEBUG("CPUID read from register:  0x%016lx", cpuid);
+
+/* parse the coresponding part_id bits */
+data->pvr = cpuid>>4&0xFFF;
+/* parse the coresponding vendor_id bits */
+data->vendor_id = cpuid>>24&0xFF;
+
+hwcaps = getauxval(AT_HWCAP);
+VIR_DEBUG("CPU flags read from register:  0x%016lx", hwcaps);
+
+if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0)
+return -1;
+
+/* shift bit map mask to parse for CPU flags */
+for (i = 0; i< MAX_CPU_FLAGS; i++) {
+if (hwcaps & BIT_SHIFTS(i)) {
+features[cpu_feature_index] = g_strdup(flag_list[i]);
+cpu_feature_index++;
+}
+}
+
+if (cpu_feature_index > 1) {
+cpu_feature_str = virStringListJoin((const char **)features, " ");
+if (!cpu_feature_str)
+goto error;
+}
+data->features = g_strdup(cpu_feature_str);
+
+return 0;
+
+ error:
+virStringListFree(features);
+return -1;
+}
+
+static int
+virCPUarmDataParseFeatures(virCPUDefPtr cpu,
+   const virCPUarmData *cpuData)
+{
+int ret = -1;
+size_t i;
+char **features;
+
+if (!cpu || !cpuData)
+return ret;
+
+if (!(features = virStringSplitCount(cpuData->features, " ",
+ 0, &cpu->nfeatures)))
+return ret;
+if (cpu->nfeatures) {
+if (VIR_ALLOC_N(cpu->features, cpu->nfeatures) < 0)
+goto error;
+
+for (i = 0; i < cpu->nfeatures; i++) {
+cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE;
+cpu->features[i].name = g_strdup(features[i]);
+}
+}
+
+ret = 0;
+
+ cleanup:
+virStringListFree(features);
+return ret;
+
+ error:
+for (i = 0; i < cpu->nfeatures; i++)
+VIR_FREE(cpu->features[i].name);
+VIR_FREE(cpu->features);
+cpu->nfeatures = 0;
+goto cleanup;
+}
+
+static int
+virCPUarmDecode(virCPUDefPtr cpu,
+const virCPUarmData *cpuData,
+virDomainCapsCPUModelsPtr models)
+{
+virCPUarmMapPtr map;
+virCPUarmModelPtr model;
+virCPUarmVendorPtr vendor = NULL;
+
+if (!cpuData || !(map = virCPUarmGetMap()))
+return -1;
+
+if (!(model = virCPUarmModelFindByPVR(map, cpuData->pvr))) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("Cannot find CPU model with PVR 0x%03lx"),
+   cpuData->pvr);
+return -1;
+}
+
+if (!virCPUModelIsAllowed(model->name, models)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("CPU model %s is not supported by hypervisor"),
+   model->name);
+return -1;
+}
+
+cpu->model = g_strdup(mod

[PATCH V4 3/5] cpu: Add helper funtions to parse vendor and model

[Zhenyu Zheng]

Add helper functions to parse vendor and model from
xml for ARM arch, and use them as callbacks when
load cpu maps.

Signed-off-by: Zhenyu Zheng 
---
 src/cpu/cpu_arm.c | 171 +-
 1 file changed, 170 insertions(+), 1 deletion(-)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index d8f571cae3..24404eac2c 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -206,6 +206,174 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt
G_GNUC_UNUSED,
 return 0;
 }

+static virCPUarmVendorPtr
+virCPUarmVendorFindByID(virCPUarmMapPtr map,
+unsigned long vendor_id)
+{
+size_t i;
+
+for (i = 0; i < map->nvendors; i++) {
+if (map->vendors[i]->value == vendor_id)
+return map->vendors[i];
+}
+
+return NULL;
+}
+
+
+static virCPUarmVendorPtr
+virCPUarmVendorFindByName(virCPUarmMapPtr map,
+  const char *name)
+{
+size_t i;
+
+for (i = 0; i < map->nvendors; i++) {
+if (STREQ(map->vendors[i]->name, name))
+return map->vendors[i];
+}
+
+return NULL;
+}
+
+
+static int
+virCPUarmVendorParse(xmlXPathContextPtr ctxt,
+   const char *name,
+   void *data)
+{
+virCPUarmMapPtr map = data;
+virCPUarmVendorPtr vendor = NULL;
+int ret = -1;
+
+if (VIR_ALLOC(vendor) < 0)
+return ret;
+
+vendor->name = g_strdup(name);
+
+if (virCPUarmVendorFindByName(map, vendor->name)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("CPU vendor %s already defined"),
+   vendor->name);
+goto cleanup;
+}
+
+if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("Missing CPU vendor value"));
+goto cleanup;
+}
+
+if (virCPUarmVendorFindByID(map, vendor->value)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("CPU vendor value 0x%2lx already defined"),
+   vendor->value);
+goto cleanup;
+}
+
+if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virCPUarmVendorFree(vendor);
+return ret;
+
+}
+
+static virCPUarmModelPtr
+virCPUarmModelFind(virCPUarmMapPtr map,
+   const char *name)
+{
+size_t i;
+
+for (i = 0; i < map->nmodels; i++) {
+if (STREQ(map->models[i]->name, name))
+return map->models[i];
+}
+
+return NULL;
+}
+
+#if defined(__aarch64__)
+static virCPUarmModelPtr
+virCPUarmModelFindByPVR(virCPUarmMapPtr map,
+unsigned long pvr)
+{
+size_t i;
+
+for (i = 0; i < map->nmodels; i++) {
+if (map->models[i]->data.pvr == pvr)
+return map->models[i];
+}
+
+return NULL;
+}
+#endif
+
+static int
+virCPUarmModelParse(xmlXPathContextPtr ctxt,
+  const char *name,
+  void *data)
+{
+virCPUarmMapPtr map = data;
+virCPUarmModel *model;
+g_autofree xmlNodePtr *nodes = NULL;
+g_autofree char *vendor = NULL;
+
+if (VIR_ALLOC(model) < 0)
+goto error;
+
+model->name = g_strdup(name);
+
+if (virCPUarmModelFind(map, model->name)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("CPU model %s already defined"),
+   model->name);
+goto error;
+}
+
+if (virXPathBoolean("boolean(./vendor)", ctxt)) {
+vendor = virXPathString("string(./vendor/@name)", ctxt);
+if (!vendor) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Invalid vendor element in CPU model %s"),
+   model->name);
+goto error;
+}
+
+if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unknown vendor %s referenced by CPU model
%s"),
+   vendor, model->name);
+goto error;
+}
+}
+
+if (!virXPathBoolean("boolean(./pvr)", ctxt)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing PVR information for CPU model %s"),
+   model->name);
+goto error;
+}
+
+if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr) <
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing or invalid PVR value in CPU model %s"),
+   model->name);
+goto error;
+}
+
+if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0)
+goto error;
+
+return 0;
+
+ error:
+virCPUarmModelFree(model);
+return -1;
+}
+
 static virCPUarmMapPtr
 virCPUarmLoadMap(void)
 {
@@ -213,7 +381,8 @@ virCPUarmLoadMap(void)

 map = virCPUarmMapNew();

-if (cpuMapLoad("arm", NULL, virCPUarmMap

[PATCH V4 2/5] cpu: Introduce ARM related structs

[Zhenyu Zheng]

Introduce vendor and model struct and related
cleanup functions for ARM cpu.

Signed-off-by: Zhenyu Zheng 
---
 src/cpu/cpu_arm.c | 75 +++
 1 file changed, 75 insertions(+)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index ee5802198f..d8f571cae3 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -1,6 +1,7 @@
 /*
  * cpu_arm.c: CPU driver for arm CPUs
  *
+ * Copyright (C) 2020 Huawei Technologies Co., Ltd.
  * Copyright (C) 2013 Red Hat, Inc.
  * Copyright (C) Canonical Ltd. 2012
  *
@@ -23,12 +24,16 @@

 #include "viralloc.h"
 #include "cpu.h"
+#include "cpu_arm.h"
 #include "cpu_map.h"
+#include "virlog.h"
 #include "virstring.h"
 #include "virxml.h"

 #define VIR_FROM_THIS VIR_FROM_CPU

+VIR_LOG_INIT("cpu.cpu_arm");
+
 static const virArch archs[] = {
 VIR_ARCH_ARMV6L,
 VIR_ARCH_ARMV7B,
@@ -36,6 +41,21 @@ static const virArch archs[] = {
 VIR_ARCH_AARCH64,
 };

+typedef struct _virCPUarmVendor virCPUarmVendor;
+typedef virCPUarmVendor *virCPUarmVendorPtr;
+struct _virCPUarmVendor {
+char *name;
+unsigned long value;
+};
+
+typedef struct _virCPUarmModel virCPUarmModel;
+typedef virCPUarmModel *virCPUarmModelPtr;
+struct _virCPUarmModel {
+char *name;
+virCPUarmVendorPtr vendor;
+virCPUarmData data;
+};
+
 typedef struct _virCPUarmFeature virCPUarmFeature;
 typedef virCPUarmFeature *virCPUarmFeaturePtr;
 struct _virCPUarmFeature {
@@ -64,6 +84,10 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUarmFeature,
virCPUarmFeatureFree);
 typedef struct _virCPUarmMap virCPUarmMap;
 typedef virCPUarmMap *virCPUarmMapPtr;
 struct _virCPUarmMap {
+size_t nvendors;
+virCPUarmVendorPtr *vendors;
+size_t nmodels;
+virCPUarmModelPtr *models;
 GPtrArray *features;
 };

@@ -81,12 +105,62 @@ virCPUarmMapNew(void)
 return map;
 }

+static void
+virCPUarmDataClear(virCPUarmData *data)
+{
+if (!data)
+return;
+
+VIR_FREE(data->features);
+}
+
+static void
+virCPUarmDataFree(virCPUDataPtr cpuData)
+{
+if (!cpuData)
+return;
+
+virCPUarmDataClear(&cpuData->data.arm);
+VIR_FREE(cpuData);
+}
+
+static void
+virCPUarmModelFree(virCPUarmModelPtr model)
+{
+if (!model)
+return;
+
+virCPUarmDataClear(&model->data);
+g_free(model->name);
+g_free(model);
+}
+
+static void
+virCPUarmVendorFree(virCPUarmVendorPtr vendor)
+{
+if (!vendor)
+return;
+
+g_free(vendor->name);
+g_free(vendor);
+}
+
 static void
 virCPUarmMapFree(virCPUarmMapPtr map)
 {
 if (!map)
 return;

+size_t i;
+
+for (i = 0; i < map->nmodels; i++)
+virCPUarmModelFree(map->models[i]);
+g_free(map->models);
+
+for (i = 0; i < map->nvendors; i++)
+virCPUarmVendorFree(map->vendors[i]);
+g_free(map->vendors);
+
 g_ptr_array_free(map->features, TRUE);

 g_free(map);
@@ -259,6 +333,7 @@ struct cpuArchDriver cpuDriverArm = {
 .compare = virCPUarmCompare,
 .decode = NULL,
 .encode = NULL,
+.dataFree = virCPUarmDataFree,
 .baseline = virCPUarmBaseline,
 .update = virCPUarmUpdate,
 .validateFeatures = virCPUarmValidateFeatures,
-- 
2.26.2

[PATCH V4 1/5] cpu: Introduce virCPUarmData to virCPUData

[Zhenyu Zheng]

Introduce virCPUarmData to virCPUData

Signed-off-by: Zhenyu Zheng 
---
 src/cpu/Makefile.inc.am |  1 +
 src/cpu/cpu.h   |  2 ++
 src/cpu/cpu_arm_data.h  | 31 +++
 3 files changed, 34 insertions(+)
 create mode 100644 src/cpu/cpu_arm_data.h

diff --git a/src/cpu/Makefile.inc.am b/src/cpu/Makefile.inc.am
index 0abeee87b6..228112a3c6 100644
--- a/src/cpu/Makefile.inc.am
+++ b/src/cpu/Makefile.inc.am
@@ -9,6 +9,7 @@ CPU_SOURCES = \
cpu/cpu_s390.h \
cpu/cpu_s390.c \
cpu/cpu_arm.h \
+   cpu/cpu_arm_data.h \
cpu/cpu_arm.c \
cpu/cpu_ppc64.h \
cpu/cpu_ppc64.c \
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index f779d2be17..ec22a183a1 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -27,6 +27,7 @@
 #include "cpu_conf.h"
 #include "cpu_x86_data.h"
 #include "cpu_ppc64_data.h"
+#include "cpu_arm_data.h"


 typedef struct _virCPUData virCPUData;
@@ -36,6 +37,7 @@ struct _virCPUData {
 union {
 virCPUx86Data x86;
 virCPUppc64Data ppc64;
+virCPUarmData arm;
 /* generic driver needs no data */
 } data;
 };
diff --git a/src/cpu/cpu_arm_data.h b/src/cpu/cpu_arm_data.h
new file mode 100644
index 00..cf12ca8c2e
--- /dev/null
+++ b/src/cpu/cpu_arm_data.h
@@ -0,0 +1,31 @@
+/*
+ * cpu_arm_data.h: 64-bit arm CPU specific data
+ *
+ * Copyright (C) 2020 Huawei Technologies Co., Ltd.
+ *
+ * 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
+ * .
+ *
+ */
+
+#pragma once
+
+#define VIR_CPU_ARM_DATA_INIT { 0 }
+
+typedef struct _virCPUarmData virCPUarmData;
+struct _virCPUarmData {
+unsigned long vendor_id;
+unsigned long pvr;
+char *features;
+};
-- 
2.26.2

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

On Tue, Apr 28, 2020 at 10:14:37PM +0800, Dr. David Alan Gilbert wrote:
> * Yan Zhao (yan.y.z...@intel.com) wrote:
> > On Mon, Apr 27, 2020 at 11:37:43PM +0800, Dr. David Alan Gilbert wrote:
> > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > On Sat, Apr 25, 2020 at 03:10:49AM +0800, Dr. David Alan Gilbert wrote:
> > > > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > > > > > On Tue, Apr 21, 2020 at 08:08:49PM +0800, Tian, Kevin wrote:
> > > > > > > > From: Yan Zhao
> > > > > > > > Sent: Tuesday, April 21, 2020 10:37 AM
> > > > > > > > 
> > > > > > > > On Tue, Apr 21, 2020 at 06:56:00AM +0800, Alex Williamson wrote:
> > > > > > > > > On Sun, 19 Apr 2020 21:24:57 -0400
> > > > > > > > > Yan Zhao  wrote:
> > > > > > > > >
> > > > > > > > > > On Fri, Apr 17, 2020 at 07:24:57PM +0800, Cornelia Huck 
> > > > > > > > > > wrote:
> > > > > > > > > > > On Fri, 17 Apr 2020 05:52:02 -0400
> > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > >
> > > > > > > > > > > > On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck 
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > On Mon, 13 Apr 2020 01:52:01 -0400
> > > > > > > > > > > > > Yan Zhao  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > This patchset introduces a migration_version 
> > > > > > > > > > > > > > attribute under sysfs
> > > > > > > > of VFIO
> > > > > > > > > > > > > > Mediated devices.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > This migration_version attribute is used to check 
> > > > > > > > > > > > > > migration
> > > > > > > > compatibility
> > > > > > > > > > > > > > between two mdev devices.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Currently, it has two locations:
> > > > > > > > > > > > > > (1) under mdev_type node,
> > > > > > > > > > > > > > which can be used even before device creation, 
> > > > > > > > > > > > > > but only for
> > > > > > > > mdev
> > > > > > > > > > > > > > devices of the same mdev type.
> > > > > > > > > > > > > > (2) under mdev device node,
> > > > > > > > > > > > > > which can only be used after the mdev devices 
> > > > > > > > > > > > > > are created, but
> > > > > > > > the src
> > > > > > > > > > > > > > and target mdev devices are not necessarily be 
> > > > > > > > > > > > > > of the same
> > > > > > > > mdev type
> > > > > > > > > > > > > > (The second location is newly added in v5, in order 
> > > > > > > > > > > > > > to keep
> > > > > > > > consistent
> > > > > > > > > > > > > > with the migration_version node for migratable 
> > > > > > > > > > > > > > pass-though
> > > > > > > > devices)
> > > > > > > > > > > > >
> > > > > > > > > > > > > What is the relationship between those two attributes?
> > > > > > > > > > > > >
> > > > > > > > > > > > (1) is for mdev devices specifically, and (2) is 
> > > > > > > > > > > > provided to keep the
> > > > > > > > same
> > > > > > > > > > > > sysfs interface as with non-mdev cases. so (2) is for 
> > > > > > > > > > > > both mdev
> > > > > > > > devices and
> > > > > > > > > > > > non-mdev devices.
> > > > > > > > > > > >
> > > > > > > > > > > > in future, if we enable vfio-pci vendor ops, (i.e. a 
> > > > > > > > > > > > non-mdev device
> > > > > > > > > > > > is binding to vfio-pci, but is able to register 
> > > > > > > > > > > > migration region and do
> > > > > > > > > > > > migration transactions from a vendor provided affiliate 
> > > > > > > > > > > > driver),
> > > > > > > > > > > > the vendor driver would export (2) directly, under 
> > > > > > > > > > > > device node.
> > > > > > > > > > > > It is not able to provide (1) as there're no mdev 
> > > > > > > > > > > > devices involved.
> > > > > > > > > > >
> > > > > > > > > > > Ok, creating an alternate attribute for non-mdev devices 
> > > > > > > > > > > makes sense.
> > > > > > > > > > > However, wouldn't that rather be a case (3)? The change 
> > > > > > > > > > > here only
> > > > > > > > > > > refers to mdev devices.
> > > > > > > > > > >
> > > > > > > > > > as you pointed below, (3) and (2) serve the same purpose.
> > > > > > > > > > and I think a possible usage is to migrate between a 
> > > > > > > > > > non-mdev device and
> > > > > > > > > > an mdev device. so I think it's better for them both to use 
> > > > > > > > > > (2) rather
> > > > > > > > > > than creating (3).
> > > > > > > > >
> > > > > > > > > An mdev type is meant to define a software compatible 
> > > > > > > > > interface, so in
> > > > > > > > > the case of mdev->mdev migration, doesn't migrating to a 
> > > > > > > > > different type
> > > > > > > > > fail the most basic of compatibility tests that we expect 
> > > > > > > > > userspace to
> > > > > > > > > perform?  IOW, if two mdev types are migration compatible, it 
> > > > > > > > > seems a
> > > > > > > > > prerequisite to that is that they provide the same software 
> > > > > > > > > interface,
> > > > > > > > > which means they should be the same mdev type.
> > > > > > > > >
> > > > > > > > > In the hybrid ca

Re: [libvirt-ci PATCH 1/4] lcitool: Tweak formatting

[Erik Skultety]

On Tue, Apr 28, 2020 at 03:37:57PM +0200, Andrea Bolognani wrote:
> This will make further changes nicer.
> 
> Signed-off-by: Andrea Bolognani 

Reviewed-by: Erik Skultety