[PATCH trivial v2 1/1] logging.html.in: fix number of output formats available

[Daniel Henrique Barboza]

There are 4 formats available (x:stderr, x:syslog:name,
x:file:file_path, x:journald), not 3. Use "the following"
instead of the actual number to avoid the need to update
the number every time a new form is added/removed.

Suggested-by: Pino Toscano 
Signed-off-by: Daniel Henrique Barboza 
---
 docs/logging.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/logging.html.in b/docs/logging.html.in
index 65c13e8a19..3ff0dd3eb6 100644
--- a/docs/logging.html.in
+++ b/docs/logging.html.in
@@ -124,7 +124,7 @@ x:name
priority level, messages that match that filter will still be logged,
while others will not. In order to see those messages, you must also 
have
an output defined that includes the priority level of your filter.
-The format for an output can be one of those 3 forms:
+The format for an output can be one of the following forms:
 
   x:stderr output goes to stderr
   x:syslog:name use syslog for the output and use the
-- 
2.26.2

Re: [PATCH trivial 1/1] logging.html.in: fix number of output formats available

[Daniel Henrique Barboza]

On 5/19/20 6:27 PM, Pino Toscano wrote:

On Tuesday, 19 May 2020 21:55:19 CEST Daniel Henrique Barboza wrote:

There are 4 formats available (x:stderr, x:syslog:name,
x:file:file_path, x:journald), not 3.

Signed-off-by: Daniel Henrique Barboza 
---
  docs/logging.html.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/logging.html.in b/docs/logging.html.in
index 65c13e8a19..9f320b439e 100644
--- a/docs/logging.html.in
+++ b/docs/logging.html.in
@@ -124,7 +124,7 @@ x:name
 priority level, messages that match that filter will still be logged,
 while others will not. In order to see those messages, you must also 
have
 an output defined that includes the priority level of your filter.
-The format for an output can be one of those 3 forms:
+The format for an output can be one of those 4 forms:


Or maybe s/those 3/the following/, so there is no need to keep the
updated count of the allowed forms (I think it doesn't matter how many
they are).



Agree. I'll reroll this one.


Thanks,


DHB

Re: [PATCH trivial 1/1] logging.html.in: fix number of output formats available

[Pino Toscano]

On Tuesday, 19 May 2020 21:55:19 CEST Daniel Henrique Barboza wrote:
> There are 4 formats available (x:stderr, x:syslog:name,
> x:file:file_path, x:journald), not 3.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  docs/logging.html.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/logging.html.in b/docs/logging.html.in
> index 65c13e8a19..9f320b439e 100644
> --- a/docs/logging.html.in
> +++ b/docs/logging.html.in
> @@ -124,7 +124,7 @@ x:name
> priority level, messages that match that filter will still be logged,
> while others will not. In order to see those messages, you must also 
> have
> an output defined that includes the priority level of your filter.
> -The format for an output can be one of those 3 forms:
> +The format for an output can be one of those 4 forms:

Or maybe s/those 3/the following/, so there is no need to keep the
updated count of the allowed forms (I think it doesn't matter how many
they are).

-- 
Pino Toscano

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

Re: Question: How do I discard any changes for the device which is set by blockdev option?

[Masayoshi Mizuma]

On Tue, May 19, 2020 at 01:41:08PM -0500, Eric Blake wrote:
> On 5/19/20 12:56 PM, Masayoshi Mizuma wrote:
> > Hello,
> > 
> > I would like to discard any changes while the qemu guest OS is done.
> > I can do that with snapshot and drive option.
> > However, snapshot option doesn't work for the device which set by
> > blockdev option like as:
> > 
> > $QEMU --enable-kvm \
> >-m 1024 \
> >-nographic \
> >-serial mon:stdio \
> >-blockdev driver=file,node-name=mydisk,filename=/mnt/fedora.qcow2 \
> >-blockdev driver=qcow2,node-name=vda,file=mydisk \
> >-device virtio-blk-pci,drive=vda,bootindex=1 \
> >-snapshot
> > 
> > I would like to use blockdev option to set the device because
> > libvirt uses blockdev option for disk element.
> > 
> > If there's no way to do so, does that make sense to get available
> > snapshot option to blockdev as well? If that makes sense, I'll try to
> > implement that.
> > 
> > As for qcow2, I think we can do such things to use qemu-img snapshot
> > command, for example save the original image and restore the image
> > after the qemu guest OS is shutdowned. However, it may be complecated
> > for user. I would like the simple way like as snapshot/drive option...
> > 
> > If I'm missing something, let me know.
> > 
> 
> Sounds like a repeat of this thread:
> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06144.html
> 
> where the consensus is yes, -blockdev and -snapshot are incompatible,
> libvirt has plans to use the  tag to behave the same as what
> -snapshot does (but no one has implemented it yet), and in the meantime, it
> is possible to force libvirt to avoid -blockdev if you still need to supply
> -snapshot behind libvirt's back.

Thank you for the info! I didn't notice the thread.
I got we should implement that feature for libvirt side, not qemu.

Thanks!
Masa

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

[PATCH trivial 1/1] logging.html.in: fix number of output formats available

[Daniel Henrique Barboza]

There are 4 formats available (x:stderr, x:syslog:name,
x:file:file_path, x:journald), not 3.

Signed-off-by: Daniel Henrique Barboza 
---
 docs/logging.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/logging.html.in b/docs/logging.html.in
index 65c13e8a19..9f320b439e 100644
--- a/docs/logging.html.in
+++ b/docs/logging.html.in
@@ -124,7 +124,7 @@ x:name
priority level, messages that match that filter will still be logged,
while others will not. In order to see those messages, you must also 
have
an output defined that includes the priority level of your filter.
-The format for an output can be one of those 3 forms:
+The format for an output can be one of those 4 forms:
 
   x:stderr output goes to stderr
   x:syslog:name use syslog for the output and use the
-- 
2.26.2

Re: [libvirt PATCH 1/6] qemu: stop checking virObjectUnref return value

[Eric Blake]

On 5/19/20 12:41 PM, Daniel P. Berrangé wrote:

Some, but not all, of the monitor event handlers check
the virObjectUnref return value to see if the domain
was disposed.

It should not be possible for this to happen, since
the functional ready holds a lock on the domain and


s/functional ready/function already/ (not everyday that a misplaced 
space still results in two valid words which might still legitimately 
appear adjacent in some other sentence...)



has only just acquired an extra reference on the
domain a few lines earlier.

Signed-off-by: Daniel P. Berrangé 
---
  src/qemu/qemu_process.c | 30 --
  1 file changed, 12 insertions(+), 18 deletions(-)


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

Re: Question: How do I discard any changes for the device which is set by blockdev option?

[Eric Blake]

On 5/19/20 12:56 PM, Masayoshi Mizuma wrote:

Hello,

I would like to discard any changes while the qemu guest OS is done.
I can do that with snapshot and drive option.
However, snapshot option doesn't work for the device which set by
blockdev option like as:

$QEMU --enable-kvm \
   -m 1024 \
   -nographic \
   -serial mon:stdio \
   -blockdev driver=file,node-name=mydisk,filename=/mnt/fedora.qcow2 \
   -blockdev driver=qcow2,node-name=vda,file=mydisk \
   -device virtio-blk-pci,drive=vda,bootindex=1 \
   -snapshot

I would like to use blockdev option to set the device because
libvirt uses blockdev option for disk element.

If there's no way to do so, does that make sense to get available
snapshot option to blockdev as well? If that makes sense, I'll try to
implement that.

As for qcow2, I think we can do such things to use qemu-img snapshot
command, for example save the original image and restore the image
after the qemu guest OS is shutdowned. However, it may be complecated
for user. I would like the simple way like as snapshot/drive option...

If I'm missing something, let me know.



Sounds like a repeat of this thread:
https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06144.html

where the consensus is yes, -blockdev and -snapshot are incompatible, 
libvirt has plans to use the  tag to behave the same as what 
-snapshot does (but no one has implemented it yet), and in the meantime, 
it is possible to force libvirt to avoid -blockdev if you still need to 
supply -snapshot behind libvirt's back.


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

[libvirt PATCH 6/6] src: make virObject inherit from GObject

[Daniel P . Berrangé]

To avoid bugs with mixing of g_object_(ref|unref) vs
virObject(Ref|Unref), we want every virObject to be
a GObject.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virobject.c | 141 +--
 src/util/virobject.h |  25 +++-
 2 files changed, 90 insertions(+), 76 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 4060d7307b..3f0bcc38ea 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -39,6 +39,7 @@ static unsigned int magicCounter = 0xCAFE;
 struct _virClass {
 virClassPtr parent;
 
+GType type;
 unsigned int magic;
 char *name;
 size_t objectSize;
@@ -46,25 +47,28 @@ struct _virClass {
 virObjectDisposeCallback dispose;
 };
 
-#define VIR_OBJECT_NOTVALID(obj) (!obj || ((obj->u.s.magic & 0x) != 
0xCAFE))
+typedef struct _virObjectPrivate virObjectPrivate;
+struct _virObjectPrivate {
+virClassPtr klass;
+};
+
+
+G_DEFINE_TYPE_WITH_PRIVATE(virObject, vir_object, G_TYPE_OBJECT)
+
+#define VIR_OBJECT_NOTVALID(obj) (!obj || !VIR_IS_OBJECT(obj))
 
 #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass) \
 do { \
 virObjectPtr obj = anyobj; \
-if (VIR_OBJECT_NOTVALID(obj)) { \
-if (!obj) \
-VIR_WARN("Object cannot be NULL"); \
-else \
-VIR_WARN("Object %p has a bad magic number %X", \
- obj, obj->u.s.magic); \
-} else { \
+if (!obj) \
+VIR_WARN("Object cannot be NULL"); \
+if (VIR_OBJECT_NOTVALID(obj)) \
 VIR_WARN("Object %p (%s) is not a %s instance", \
- anyobj, obj->klass->name, #objclass); \
-} \
+ anyobj, g_type_name_from_instance((void*)anyobj), 
#objclass); \
 } while (0)
 
 
-static virClassPtr virObjectClass;
+static virClassPtr virObjectClassImpl;
 static virClassPtr virObjectLockableClass;
 static virClassPtr virObjectRWLockableClass;
 
@@ -74,17 +78,17 @@ static void virObjectRWLockableDispose(void *anyobj);
 static int
 virObjectOnceInit(void)
 {
-if (!(virObjectClass = virClassNew(NULL,
-   "virObject",
-   sizeof(virObject),
-   0,
-   NULL)))
+if (!(virObjectClassImpl = virClassNew(NULL,
+   "virObject",
+   sizeof(virObject),
+   0,
+   NULL)))
 return -1;
 
-if (!VIR_CLASS_NEW(virObjectLockable, virObjectClass))
+if (!VIR_CLASS_NEW(virObjectLockable, virObjectClassImpl))
 return -1;
 
-if (!VIR_CLASS_NEW(virObjectRWLockable, virObjectClass))
+if (!VIR_CLASS_NEW(virObjectRWLockable, virObjectClassImpl))
 return -1;
 
 return 0;
@@ -104,7 +108,7 @@ virClassForObject(void)
 if (virObjectInitialize() < 0)
 return NULL;
 
-return virObjectClass;
+return virObjectClassImpl;
 }
 
 
@@ -138,6 +142,14 @@ virClassForObjectRWLockable(void)
 }
 
 
+static void virClassDummyInit(void *klass G_GNUC_UNUSED)
+{
+}
+
+static void virObjectDummyInit(void *obj G_GNUC_UNUSED)
+{
+}
+
 /**
  * virClassNew:
  * @parent: the parent class
@@ -177,25 +189,26 @@ virClassNew(virClassPtr parent,
 return NULL;
 }
 
-if (VIR_ALLOC(klass) < 0)
-goto error;
-
+klass = g_new0(virClass, 1);
 klass->parent = parent;
 klass->magic = g_atomic_int_add(, 1);
-if (klass->magic > 0xCAFE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("too many object classes defined"));
-goto error;
-}
 klass->name = g_strdup(name);
 klass->objectSize = objectSize;
+if (parent == NULL) {
+klass->type = vir_object_get_type();
+} else {
+klass->type =
+g_type_register_static_simple(parent->type,
+  name,
+  sizeof(virObjectClass),
+  (GClassInitFunc)virClassDummyInit,
+  objectSize,
+  
(GInstanceInitFunc)virObjectDummyInit,
+  0);
+}
 klass->dispose = dispose;
 
 return klass;
-
- error:
-VIR_FREE(klass);
-return NULL;
 }
 
 
@@ -237,17 +250,13 @@ void *
 virObjectNew(virClassPtr klass)
 {
 virObjectPtr obj = NULL;
+virObjectPrivate *priv;
 
-if (VIR_ALLOC_VAR(obj,
-  char,
-  klass->objectSize - sizeof(virObject)) < 0)
-return NULL;
-
-obj->u.s.magic = klass->magic;
-obj->klass = klass;
-g_atomic_int_set(>u.s.refs, 1);
+obj = g_object_new(klass->type, NULL);
 
-PROBE(OBJECT_NEW, "obj=%p 

[libvirt PATCH 1/6] qemu: stop checking virObjectUnref return value

[Daniel P . Berrangé]

Some, but not all, of the monitor event handlers check
the virObjectUnref return value to see if the domain
was disposed.

It should not be possible for this to happen, since
the functional ready holds a lock on the domain and
has only just acquired an extra reference on the
domain a few lines earlier.

Signed-off-by: Daniel P. Berrangé 
---
 src/qemu/qemu_process.c | 30 --
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f7f6793113..51a086031d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -307,7 +307,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon,
 processEvent->vm = virObjectRef(vm);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-ignore_value(virObjectUnref(vm));
+virObjectUnref(vm);
 qemuProcessEventFree(processEvent);
 goto cleanup;
 }
@@ -840,15 +840,13 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon 
G_GNUC_UNUSED,
  */
 processEvent->vm = virObjectRef(vm);
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) 
{
-if (!virObjectUnref(vm))
-vm = NULL;
+virObjectUnref(vm);
 qemuProcessEventFree(processEvent);
 }
 }
 }
 
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
 virObjectEventStateQueue(driver->domainEventState, watchdogEvent);
 virObjectEventStateQueue(driver->domainEventState, lifecycleEvent);
 
@@ -977,7 +975,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon G_GNUC_UNUSED,
 processEvent->status = status;
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-ignore_value(virObjectUnref(vm));
+virObjectUnref(vm);
 goto cleanup;
 }
 
@@ -1039,7 +1037,7 @@ qemuProcessHandleJobStatusChange(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 processEvent->data = virObjectRef(job);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-ignore_value(virObjectUnref(vm));
+virObjectUnref(vm);
 goto cleanup;
 }
 
@@ -1342,14 +1340,12 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 processEvent->vm = virObjectRef(vm);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-if (!virObjectUnref(vm))
-vm = NULL;
+virObjectUnref(vm);
 qemuProcessEventFree(processEvent);
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
 
 return 0;
 }
@@ -1383,7 +1379,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 processEvent->vm = virObjectRef(vm);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-ignore_value(virObjectUnref(vm));
+virObjectUnref(vm);
 goto error;
 }
 
@@ -1554,7 +1550,7 @@ qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 processEvent->vm = virObjectRef(vm);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-ignore_value(virObjectUnref(vm));
+virObjectUnref(vm);
 goto error;
 }
 
@@ -1593,7 +1589,7 @@ qemuProcessHandleSerialChanged(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 processEvent->vm = virObjectRef(vm);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-ignore_value(virObjectUnref(vm));
+virObjectUnref(vm);
 goto error;
 }
 
@@ -1873,14 +1869,12 @@ qemuProcessHandleGuestCrashloaded(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 processEvent->vm = virObjectRef(vm);
 
 if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-if (!virObjectUnref(vm))
-vm = NULL;
+virObjectUnref(vm);
 qemuProcessEventFree(processEvent);
 }
 
  cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
 
 return 0;
 }
-- 
2.24.1

[libvirt PATCH 2/6] src: make virObjectUnref return void

[Daniel P . Berrangé]

To prepare for a conversion to GObject, we need virObjectUnref
to have the same API design as g_object_unref, which means it
needs to be void.

A few places do actually care about the return value though,
and in these cases a thread local flag is used to determine
if the dispose method was invoked.

Signed-off-by: Daniel P. Berrangé 
---
 src/admin/libvirt-admin.c   |  4 +++-
 src/datatypes.c | 26 +
 src/datatypes.h |  6 ++
 src/interface/interface_backend_netcf.c |  7 +--
 src/libvirt.c   |  4 +++-
 src/qemu/qemu_domain.c  |  4 +++-
 src/qemu/qemu_monitor.c | 14 +
 src/qemu/qemu_monitor.h |  3 +++
 src/test/test_driver.c  |  8 ++--
 src/util/virfdstream.c  |  6 +-
 src/util/virobject.c|  9 ++---
 src/util/virobject.h|  2 +-
 src/vbox/vbox_common.c  | 10 --
 13 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 835b5560d2..1d4ac51296 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -295,7 +295,9 @@ virAdmConnectClose(virAdmConnectPtr conn)
 
 virCheckAdmConnectReturn(conn, -1);
 
-if (!virObjectUnref(conn))
+virAdmConnectWatchDispose();
+virObjectUnref(conn);
+if (virAdmConnectWasDisposed())
 return 0;
 return 1;
 }
diff --git a/src/datatypes.c b/src/datatypes.c
index 552115c7a3..1db38c5aa6 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -76,6 +76,9 @@ virClassPtr virAdmClientClass;
 static void virAdmServerDispose(void *obj);
 static void virAdmClientDispose(void *obj);
 
+static __thread bool connectDisposed;
+static __thread bool admConnectDisposed;
+
 static int
 virDataTypesOnceInit(void)
 {
@@ -133,6 +136,27 @@ virGetConnect(void)
 return virObjectLockableNew(virConnectClass);
 }
 
+
+void virConnectWatchDispose(void)
+{
+connectDisposed = false;
+}
+
+bool virConnectWasDisposed(void)
+{
+return connectDisposed;
+}
+
+void virAdmConnectWatchDispose(void)
+{
+admConnectDisposed = false;
+}
+
+bool virAdmConnectWasDisposed(void)
+{
+return admConnectDisposed;
+}
+
 /**
  * virConnectDispose:
  * @obj: the hypervisor connection to release
@@ -145,6 +169,7 @@ virConnectDispose(void *obj)
 {
 virConnectPtr conn = obj;
 
+connectDisposed = true;
 if (conn->driver)
 conn->driver->connectClose(conn);
 
@@ -1092,6 +1117,7 @@ virAdmConnectDispose(void *obj)
 {
 virAdmConnectPtr conn = obj;
 
+admConnectDisposed = true;
 if (conn->privateDataFreeFunc)
 conn->privateDataFreeFunc(conn);
 
diff --git a/src/datatypes.h b/src/datatypes.h
index 2d0407f7ec..d58429ad6c 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -843,6 +843,12 @@ virAdmClientPtr virAdmGetClient(virAdmServerPtr srv,
 unsigned long long timestamp,
 unsigned int transport);
 
+/* Thread local to watch if an ObjectUnref causes a Dispoe */
+void virConnectWatchDispose(void);
+bool virConnectWasDisposed(void);
+void virAdmConnectWatchDispose(void);
+bool virAdmConnectWasDisposed(void);
+
 virConnectCloseCallbackDataPtr virNewConnectCloseCallbackData(void);
 void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close,
  virConnectPtr conn,
diff --git a/src/interface/interface_backend_netcf.c 
b/src/interface/interface_backend_netcf.c
index dd0c1481d9..f30829442d 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -148,12 +148,7 @@ netcfStateCleanup(void)
 if (!driver)
 return -1;
 
-if (virObjectUnref(driver)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Attempt to close netcf state driver "
- "with open connections"));
-return -1;
-}
+virObjectUnref(driver);
 driver = NULL;
 return 0;
 }
diff --git a/src/libvirt.c b/src/libvirt.c
index 76bf1fa677..b2d0ba3d23 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1277,7 +1277,9 @@ virConnectClose(virConnectPtr conn)
 
 virCheckConnectReturn(conn, -1);
 
-if (!virObjectUnref(conn))
+virConnectWatchDispose();
+virObjectUnref(conn);
+if (virConnectWasDisposed())
 return 0;
 return 1;
 }
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d0528dbfe0..bb77cd38d3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6681,8 +6681,10 @@ qemuDomainObjExitMonitorInternal(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = obj->privateData;
 bool hasRefs;
 
-hasRefs = virObjectUnref(priv->mon);
+qemuMonitorWatchDispose();
+virObjectUnref(priv->mon);
 
+hasRefs = 

[libvirt PATCH 5/6] src: don't use VIR_FREE on an object allocation

[Daniel P . Berrangé]

Memory allocated using g_object_new must never be released using
VIR_FREE/g_free because g_object_new uses a special allocation
strategy internally.

Signed-off-by: Daniel P. Berrangé 
---
 src/rpc/virnettlscontext.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 02c17124a1..ced0cbdcd8 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -750,12 +750,9 @@ static virNetTLSContextPtr virNetTLSContextNew(const char 
*cacert,
 return ctxt;
 
  error:
+virObjectUnref(ctxt);
 if (isServer)
 gnutls_dh_params_deinit(ctxt->dhParams);
-if (ctxt->x509cred)
-gnutls_certificate_free_credentials(ctxt->x509cred);
-VIR_FREE(ctxt->priority);
-VIR_FREE(ctxt);
 return NULL;
 }
 
-- 
2.24.1

[libvirt PATCH 4/6] src: don't include ref count in debug messages / probes

[Daniel P . Berrangé]

The ref count will be private to the GObject base class
and we must not peek at it, even for debugging messages.

Signed-off-by: Daniel P. Berrangé 
---
 src/admin/libvirt-admin.c   | 3 +--
 src/libvirt-domain-checkpoint.c | 3 +--
 src/libvirt-domain-snapshot.c   | 3 +--
 src/libvirt-domain.c| 2 +-
 src/libvirt-host.c  | 2 +-
 src/libvirt-interface.c | 2 +-
 src/libvirt-network.c   | 6 ++
 src/libvirt-nodedev.c   | 2 +-
 src/libvirt-nwfilter.c  | 6 ++
 src/libvirt-secret.c| 3 +--
 src/libvirt-storage.c   | 4 ++--
 src/libvirt-stream.c| 3 +--
 src/libvirt_qemu_probes.d   | 8 
 src/qemu/qemu_monitor.c | 7 ++-
 14 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 1d4ac51296..b7f21275cb 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -322,8 +322,7 @@ virAdmConnectClose(virAdmConnectPtr conn)
 int
 virAdmConnectRef(virAdmConnectPtr conn)
 {
-VIR_DEBUG("conn=%p refs=%d", conn,
-  conn ? conn->parent.parent.u.s.refs : 0);
+VIR_DEBUG("conn=%p", conn);
 
 virResetLastError();
 virCheckAdmConnectReturn(conn, -1);
diff --git a/src/libvirt-domain-checkpoint.c b/src/libvirt-domain-checkpoint.c
index 432c2d5a52..50627c486c 100644
--- a/src/libvirt-domain-checkpoint.c
+++ b/src/libvirt-domain-checkpoint.c
@@ -535,8 +535,7 @@ virDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
 int
 virDomainCheckpointRef(virDomainCheckpointPtr checkpoint)
 {
-VIR_DEBUG("checkpoint=%p, refs=%d", checkpoint,
-  checkpoint ? checkpoint->parent.u.s.refs : 0);
+VIR_DEBUG("checkpoint=%p", checkpoint);
 
 virResetLastError();
 
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 33593e11e9..f856e5b9b8 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -1080,8 +1080,7 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
 int
 virDomainSnapshotRef(virDomainSnapshotPtr snapshot)
 {
-VIR_DEBUG("snapshot=%p, refs=%d", snapshot,
-  snapshot ? snapshot->parent.u.s.refs : 0);
+VIR_DEBUG("snapshot=%p", snapshot);
 
 virResetLastError();
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 37f864b7b0..60b5e65fc3 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -590,7 +590,7 @@ virDomainFree(virDomainPtr domain)
 int
 virDomainRef(virDomainPtr domain)
 {
-VIR_DOMAIN_DEBUG(domain, "refs=%d", domain ? domain->parent.u.s.refs : 0);
+VIR_DOMAIN_DEBUG(domain);
 
 virResetLastError();
 
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index bc3d1d2803..07d13585f4 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -51,7 +51,7 @@ VIR_LOG_INIT("libvirt.host");
 int
 virConnectRef(virConnectPtr conn)
 {
-VIR_DEBUG("conn=%p refs=%d", conn, conn ? conn->parent.parent.u.s.refs : 
0);
+VIR_DEBUG("conn=%p", conn);
 
 virResetLastError();
 
diff --git a/src/libvirt-interface.c b/src/libvirt-interface.c
index 2d2df68131..5eb5980483 100644
--- a/src/libvirt-interface.c
+++ b/src/libvirt-interface.c
@@ -642,7 +642,7 @@ virInterfaceDestroy(virInterfacePtr iface, unsigned int 
flags)
 int
 virInterfaceRef(virInterfacePtr iface)
 {
-VIR_DEBUG("iface=%p refs=%d", iface, iface ? iface->parent.u.s.refs : 0);
+VIR_DEBUG("iface=%p", iface);
 
 virResetLastError();
 
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index 09e24fb0a8..f691b672c7 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -679,8 +679,7 @@ virNetworkFree(virNetworkPtr network)
 int
 virNetworkRef(virNetworkPtr network)
 {
-VIR_DEBUG("network=%p refs=%d", network,
-  network ? network->parent.u.s.refs : 0);
+VIR_DEBUG("network=%p", network);
 
 virResetLastError();
 
@@ -1714,8 +1713,7 @@ virNetworkPortFree(virNetworkPortPtr port)
 int
 virNetworkPortRef(virNetworkPortPtr port)
 {
-VIR_DEBUG("port=%p refs=%d", port,
-  port ? port->parent.u.s.refs : 0);
+VIR_DEBUG("port=%p", port);
 
 virResetLastError();
 
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index dce46b7181..cdec123568 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -477,7 +477,7 @@ virNodeDeviceFree(virNodeDevicePtr dev)
 int
 virNodeDeviceRef(virNodeDevicePtr dev)
 {
-VIR_DEBUG("dev=%p refs=%d", dev, dev ? dev->parent.u.s.refs : 0);
+VIR_DEBUG("dev=%p", dev);
 
 virResetLastError();
 
diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c
index d28220db8a..e299385895 100644
--- a/src/libvirt-nwfilter.c
+++ b/src/libvirt-nwfilter.c
@@ -503,8 +503,7 @@ virNWFilterGetXMLDesc(virNWFilterPtr nwfilter, unsigned int 
flags)
 int
 virNWFilterRef(virNWFilterPtr nwfilter)
 {
-VIR_DEBUG("nwfilter=%p refs=%d", nwfilter,
-  nwfilter ? nwfilter->parent.u.s.refs : 0);
+

[libvirt PATCH 3/6] test: allocate numa cells separately from driver

[Daniel P . Berrangé]

GObject has an arbitrary limit on the object struct size of 0x
bytes. It is expected that any large fields be separately allocated.

Signed-off-by: Daniel P. Berrangé 
---
 src/test/test_driver.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 3a085003e2..e8bfcd78d2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -92,7 +92,8 @@ struct _testAuth {
 typedef struct _testAuth testAuth;
 typedef struct _testAuth *testAuthPtr;
 
-struct _testDriver {
+struct _testDriver
+{
 virObjectLockable parent;
 
 virNodeInfo nodeInfo;
@@ -102,7 +103,7 @@ struct _testDriver {
 virStoragePoolObjListPtr pools;
 virNodeDeviceObjListPtr devs;
 int numCells;
-testCell cells[MAX_CELLS];
+testCell *cells;
 size_t numAuths;
 testAuthPtr auths;
 
@@ -171,6 +172,7 @@ testDriverDispose(void *obj)
 g_free(driver->auths[i].username);
 g_free(driver->auths[i].password);
 }
+g_free(driver->cells);
 g_free(driver->auths);
 
 testDriverDisposed = true;
@@ -1353,6 +1355,7 @@ testOpenDefault(virConnectPtr conn)
 
 /* Numa setup */
 privconn->numCells = 2;
+privconn->cells = g_new0(testCell, privconn->numCells);
 for (i = 0; i < privconn->numCells; i++) {
 privconn->cells[i].numCpus = 8;
 privconn->cells[i].mem = (i + 1) * 2048 * 1024;
-- 
2.24.1

[libvirt PATCH 0/6] src: make virObject inherit from GObject

[Daniel P . Berrangé]

This series attempts to make the conversion from virObject to
GObject less dangerous, by making the former inherit from the
latter.

The way we setup inheritance with virObject means we have to
make some moderately gross games with registering virClass
instances as GObjectClass instances. Empirically it seems to
work and pass the basic TCK test suite.

Ultimately virObject should be deleted entirely, so the gross
bits shouldn't live too long.

Daniel P. Berrangé (6):
  qemu: stop checking virObjectUnref return value
  src: make virObjectUnref return void
  test: allocate numa cells separately from driver
  src: don't include ref count in debug messages / probes
  src: don't use VIR_FREE on an object allocation
  src: make virObject inherit from GObject

 src/admin/libvirt-admin.c   |   7 +-
 src/datatypes.c |  26 
 src/datatypes.h |   6 +
 src/interface/interface_backend_netcf.c |   7 +-
 src/libvirt-domain-checkpoint.c |   3 +-
 src/libvirt-domain-snapshot.c   |   3 +-
 src/libvirt-domain.c|   2 +-
 src/libvirt-host.c  |   2 +-
 src/libvirt-interface.c |   2 +-
 src/libvirt-network.c   |   6 +-
 src/libvirt-nodedev.c   |   2 +-
 src/libvirt-nwfilter.c  |   6 +-
 src/libvirt-secret.c|   3 +-
 src/libvirt-storage.c   |   4 +-
 src/libvirt-stream.c|   3 +-
 src/libvirt.c   |   4 +-
 src/libvirt_qemu_probes.d   |   8 +-
 src/qemu/qemu_domain.c  |   4 +-
 src/qemu/qemu_monitor.c |  21 +++-
 src/qemu/qemu_monitor.h |   3 +
 src/qemu/qemu_process.c |  30 ++---
 src/rpc/virnettlscontext.c  |   5 +-
 src/test/test_driver.c  |  15 ++-
 src/util/virfdstream.c  |   6 +-
 src/util/virobject.c| 150 ++--
 src/util/virobject.h|  27 ++---
 src/vbox/vbox_common.c  |  10 +-
 27 files changed, 210 insertions(+), 155 deletions(-)

-- 
2.24.1

Re: [libvirt PATCH 3/6] hostcpu: Implement virHostCPUGetSignature for x86

[Ján Tomko]

On a Monday in 2020, Jiri Denemark wrote:

Signed-off-by: Jiri Denemark 
---
src/util/virhostcpu.c | 50 +--
.../linux-x86_64-test1.signature  |  1 +
.../linux-x86_64-test2.signature  |  1 +
.../linux-x86_64-test3.signature  |  1 +
.../linux-x86_64-test4.signature  |  1 +
.../linux-x86_64-test5.signature  |  1 +
.../linux-x86_64-test6.signature  |  1 +
.../linux-x86_64-test7.signature  |  1 +
.../linux-x86_64-test8.signature  |  1 +
.../linux-x86_64-with-die.signature   |  1 +
10 files changed, 56 insertions(+), 3 deletions(-)
create mode 100644 tests/virhostcpudata/linux-x86_64-test1.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test2.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test3.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test4.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test5.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test6.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test7.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test8.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-with-die.signature

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index bfef022f64..851c0015f7 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1418,10 +1418,54 @@ virHostCPUGetTscInfo(void)
  (defined(__linux__) || defined(__FreeBSD__)) */

int
-virHostCPUReadSignature(virArch arch G_GNUC_UNUSED,
-FILE *cpuinfo G_GNUC_UNUSED,
-char **signature G_GNUC_UNUSED)
+virHostCPUReadSignature(virArch arch,
+FILE *cpuinfo,
+char **signature)
{
+size_t lineLen = 1024;
+g_autofree char *line = g_new0(char, lineLen);
+g_autofree char *vendor = NULL;
+g_autofree char *name = NULL;
+g_autofree char *family = NULL;
+g_autofree char *model = NULL;
+g_autofree char *stepping = NULL;
+
+if (!ARCH_IS_X86(arch))
+return 0;
+
+while (fgets(line, lineLen, cpuinfo)) {
+   g_auto(GStrv) parts = g_strsplit(line, ": ", 2);


Indendation


+
+if (g_strv_length(parts) != 2)
+continue;
+
+g_strstrip(parts[0]);
+g_strstrip(parts[1]);
+
+if (STREQ(parts[0], "vendor_id")) {
+if (!vendor)
+vendor = g_steal_pointer([1]);
+} else if (STREQ(parts[0], "model name")) {
+if (!name)
+name = g_steal_pointer([1]);
+} else if (STREQ(parts[0], "cpu family")) {
+if (!family)
+family = g_steal_pointer([1]);
+} else if (STREQ(parts[0], "model")) {
+if (!model)
+model = g_steal_pointer([1]);
+} else if (STREQ(parts[0], "stepping")) {
+if (!stepping)
+stepping = g_steal_pointer([1]);
+}
+
+if (vendor && name && family && model && stepping) {
+*signature = g_strdup_printf("%s, %s, family: %s, model: %s, stepping: 
%s",
+ vendor, name, family, model, 
stepping);
+return 0;
+}


signature.asc
Description: PGP signature

Re: [libvirt PATCH 0/6] qemu: Invalidate capabilities when host CPU changes

[Ján Tomko]

On a Monday in 2020, Jiri Denemark wrote:

The host CPU related info stored in the capabilities cache is no longer
valid after the host CPU changes. This is not a frequent situation in
real world, but it can easily happen in nested scenarios when a disk
image is started with various CPUs.

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

Jiri Denemark (6):
 util: Define g_autoptr callback for FILE
 hostcpu: Introduce virHostCPUGetSignature
 hostcpu: Implement virHostCPUGetSignature for x86
 hostcpu: Implement virHostCPUGetSignature for ppc64
 hostcpu: Implement virHostCPUGetSignature for s390
 qemu: Invalidate capabilities when host CPU changes

src/libvirt_private.syms  |   2 +
src/qemu/qemu_capabilities.c  |  24 
src/qemu/qemu_capspriv.h  |   1 +
src/util/virfile.h|   1 +
src/util/virhostcpu.c | 111 ++
src/util/virhostcpu.h |   2 +
src/util/virhostcpupriv.h |   4 +
tests/qemucapsprobe.c |   2 +-
.../linux-ppc64-deconf-cpus.signature |   1 +
.../linux-ppc64-subcores1.signature   |   1 +
.../linux-ppc64-subcores2.signature   |   1 +
.../linux-ppc64-subcores3.signature   |   1 +
.../linux-s390x-with-frequency.signature  |   1 +
.../linux-x86_64-test1.signature  |   1 +
.../linux-x86_64-test2.signature  |   1 +
.../linux-x86_64-test3.signature  |   1 +
.../linux-x86_64-test4.signature  |   1 +
.../linux-x86_64-test5.signature  |   1 +
.../linux-x86_64-test6.signature  |   1 +
.../linux-x86_64-test7.signature  |   1 +
.../linux-x86_64-test8.signature  |   1 +
.../linux-x86_64-with-die.signature   |   1 +
tests/virhostcputest.c|  42 ++-
23 files changed, 201 insertions(+), 2 deletions(-)
create mode 100644 tests/virhostcpudata/linux-ppc64-deconf-cpus.signature
create mode 100644 tests/virhostcpudata/linux-ppc64-subcores1.signature
create mode 100644 tests/virhostcpudata/linux-ppc64-subcores2.signature
create mode 100644 tests/virhostcpudata/linux-ppc64-subcores3.signature
create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test1.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test2.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test3.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test4.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test5.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test6.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test7.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-test8.signature
create mode 100644 tests/virhostcpudata/linux-x86_64-with-die.signature



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

"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(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[libvirt PATCH 4/4] scripts: emit raw enum value in API build description

[Daniel P . Berrangé]

Currently the value for an enum is only emitted if it is a plain
string. If the enum is an integer or hex value, or a complex code block,
it is omitted from the API build. This fixes that by emitting the raw
value if no string value is present.

With this change:

  
  
  
  ...snip...

  
  
  
  ...snip...

Signed-off-by: Daniel P. Berrangé 
---
 scripts/apibuild.py | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/apibuild.py b/scripts/apibuild.py
index 9faf15a75e..d63489ba62 100755
--- a/scripts/apibuild.py
+++ b/scripts/apibuild.py
@@ -1027,11 +1027,14 @@ class CParser:
 return token
 
 strValue = None
+rawValue = None
 if len(lst) == 1 and lst[0][0] == '"' and lst[0][-1] == '"':
 strValue = lst[0][1:-1]
+else:
+rawValue = " ".join(lst)
 (args, desc) = self.parseMacroComment(name, not self.is_header)
 self.index_add(name, self.filename, not self.is_header,
-   "macro", (args, desc, params, strValue))
+   "macro", (args, desc, params, strValue, 
rawValue))
 return token
 
 #
@@ -2178,13 +2181,16 @@ class docBuilder:
 desc = None
 params = None
 strValue = None
+rawValue = None
 else:
-(args, desc, params, strValue) = id.info
+(args, desc, params, strValue, rawValue) = id.info
 
 if params is not None:
 output.write(" params='%s'" % params)
 if strValue is not None:
 output.write(" string='%s'" % strValue)
+else:
+output.write(" raw='%s'" % rawValue)
 output.write(">\n")
 
 if desc is not None and desc != "":
-- 
2.26.2

[libvirt PATCH 1/4] scripts: use UTF-8 for API XML files

[Daniel P . Berrangé]

The build system will be running in UTF-8 locale, so any content in the
API XML files will also end up being UTF-8, not ISO-8859-1.

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

diff --git a/scripts/apibuild.py b/scripts/apibuild.py
index 05a169c30d..b13b5db644 100755
--- a/scripts/apibuild.py
+++ b/scripts/apibuild.py
@@ -2333,7 +2333,7 @@ class docBuilder:
 if not quiet:
 print("Saving XML description %s" % (filename))
 output = open(filename, "w")
-output.write('\n')
+output.write('\n')
 output.write("\n" % self.name)
 output.write("  \n")
 headers = sorted(self.headers.keys())
-- 
2.26.2

[libvirt PATCH 3/4] scripts: emit enum parameters in API build description

[Daniel P . Berrangé]

Currently the information about enums in the API document lacks any
mention of parameters, so it is impossible to tell what kind of enum
declaration is present in the libvirt API header. With this change

  
  
  ...snip...

becomes

  
  
  ...snip...

Signed-off-by: Daniel P. Berrangé 
---
 scripts/apibuild.py | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/scripts/apibuild.py b/scripts/apibuild.py
index 68c588d8b6..9faf15a75e 100755
--- a/scripts/apibuild.py
+++ b/scripts/apibuild.py
@@ -1013,10 +1013,12 @@ class CParser:
token[1][0] != '#'):
 lst.append(token[1])
 token = self.lexer.token()
-try:
-name = name.split('(')[0]
-except Exception:
-pass
+
+paramStart = name.find("(")
+params = None
+if paramStart != -1:
+params = name[paramStart+1:-1]
+name = name[0:paramStart]
 
 # skip hidden macros
 if name in hidden_macros:
@@ -1029,7 +1031,7 @@ class CParser:
 strValue = lst[0][1:-1]
 (args, desc) = self.parseMacroComment(name, not self.is_header)
 self.index_add(name, self.filename, not self.is_header,
-   "macro", (args, desc, strValue))
+   "macro", (args, desc, params, strValue))
 return token
 
 #
@@ -2174,10 +2176,13 @@ class docBuilder:
 if id.info is None:
 args = []
 desc = None
+params = None
 strValue = None
 else:
-(args, desc, strValue) = id.info
+(args, desc, params, strValue) = id.info
 
+if params is not None:
+output.write(" params='%s'" % params)
 if strValue is not None:
 output.write(" string='%s'" % strValue)
 output.write(">\n")
-- 
2.26.2

[libvirt PATCH 2/4] scripts: fix tokenizing of enum parameters in API builder

[Daniel P . Berrangé]

The API build script tokenizes enums declarations by first splitting on
whitespace. This is unhelpful as it means an enum

 # define VIR_USE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] |= (1 << ((cpu) % 8)))

Gets tokenized as

  #define
  VIR_USE_CPU(cpumap,
  cpu)
  ((cpumap)[(cpu)
  /
  8]
  |=
  (1
  <<
  ((cpu)
  %
  8)))

With this change, the set of parameters are all merged into the first
token:

  #define
  VIR_USE_CPU(cpumap,cpu)
  ((cpumap)[(cpu)
  /
  8]
  |=
  (1
  <<
  ((cpu)
  %
  8)))

which is more convenient to process later on in the script.

Signed-off-by: Daniel P. Berrangé 
---
 scripts/apibuild.py | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/scripts/apibuild.py b/scripts/apibuild.py
index b13b5db644..68c588d8b6 100755
--- a/scripts/apibuild.py
+++ b/scripts/apibuild.py
@@ -494,6 +494,28 @@ class CLexer:
 if self.tokens[0][1] == "#":
 self.tokens[0] = ('preproc', "#" + self.tokens[1][1])
 del self.tokens[1]
+
+if self.tokens[0][1] == "#define" and "(" in self.tokens[1][1]:
+newtokens = [self.tokens[0]]
+
+endArg = self.tokens[1][1].find(")")
+if endArg != -1:
+extra = self.tokens[1][1][endArg+1:]
+name = self.tokens[1][1][0:endArg+1]
+newtokens.append(('preproc', name))
+if extra != "":
+newtokens.append(('preproc', extra))
+else:
+name = self.tokens[1][1]
+for token in self.tokens[2:]:
+if name is not None:
+name = name + token[1]
+if ")" in token[1]:
+newtokens.append(('preproc', name))
+name = None
+else:
+newtokens.append(token)
+self.tokens = newtokens
 break
 nline = len(line)
 if line[0] == '"' or line[0] == "'":
-- 
2.26.2

[libvirt PATCH 0/4] scripts: improve API docs handing of enums

[Daniel P . Berrangé]

In attempting to refactor the Go API bindings to be partially
auto-generated, I'm encountering missing information in the enum
descriptions.

The effects of this series are shown in this diff:

--- libvirt-api.xml 2020-05-19 12:47:22.003489289 +0100
+++ docs/libvirt-api.xml2020-05-19 12:47:27.156476186 +0100
@@ -1,4 +1,4 @@
-
+
 
   
 
@@ -2089,13 +2089,13 @@
 
   
   
-
+
   
   
   
   
 
-
+
   
 
 
@@ -2125,25 +2125,25 @@
 
   
 
-
+
   
   
   
   
   
+
   
   
 
-
+
   
   
   
   
   
 
-
+
   
   
   
@@ -2184,7 +2184,7 @@
 
   
 
-
+
   
 
 
@@ -2262,7 +2262,7 @@
 
   
 
-
+
   
 
 
@@ -2301,7 +2301,7 @@
 
   
 
-
+
   
 
 
@@ -2421,7 +2421,7 @@
 
   
 
-
+
   
 
 
@@ -2430,7 +2430,7 @@
 
   
 
 
@@ -2487,10 +2487,10 @@
 
   
 
-
+
   
 
-
+
   
 
 
@@ -2592,13 +2592,13 @@
 
   
 
-
+
   
   
   
   
 
-
+
   
 
 
@@ -2658,7 +2658,7 @@
 
   
 
-
+
   
 
 
@@ -2682,11 +2682,11 @@
 
   
 
-
+
   
   
 
-
+
   <=
/info>
 
 
@@ -2707,7 +2707,7 @@
 
   
 
-
+
   
 
 
@@ -2740,7 +2740,7 @@
 
   
 
-
+
   
 
-
+
   
 
-
+
   
 
-
+
   
 
-
+
   
 
-
+
   
 
-
+
   
   
   
 
-
+
   
   
   
 
-
+
   
 
-
+
   
 
-
+
 
-
+
 
-
+
 
 
 

Daniel P. Berrang=C3=A9 (4):
  scripts: use UTF-8 for API XML files
  scripts: fix tokenizing of enum parameters in API builder
  scripts: emit enum parameters in API build description
  scripts: emit raw enum value in API build description

 scripts/apibuild.py | 47 ++---
 1 file changed, 40 insertions(+), 7 deletions(-)

--=20
2.26.2

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(+)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

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

[Ján Tomko]

On a Wednesday in 2020, Peter Krempa wrote:

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(-)

@@ -508,16 +515,22 @@ testQEMUSchemaValidate(virJSONValuePtr obj,
 * -1 if it does not and -2 if there is a problem with the schema or with
 *  internals.
 *
+ * @alllowRemoved should generally be used only if it's certain that there's a


*allow


+ * replacement of @command in place.
+ *
 * @debug is filled with information regarding the validation process
 */
int
testQEMUSchemaValidateCommand(const char *command,
  virJSONValuePtr arguments,
  virHashTablePtr schema,


Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH] qemu: do not allow /dev/rtc or /dev/hpet access via the devices cgroup

[Paolo Bonzini]

On 19/05/20 10:58, Michal Privoznik wrote:
> 
> Ah, could it be because of the stray comma? From qemu.conf:
> 
> #cgroup_device_acl = [
> #    "/dev/null", "/dev/full", "/dev/zero",
> #    "/dev/random", "/dev/urandom",
> #    "/dev/ptmx", "/dev/kvm",
> #]
> 
> Let me check if removing the comma after /dev/kvm fixes the build.

It does.  I forgot to commit it, sorry. :(

Though perhaps accepting the trailing comma is better, which would be
something like the following untested patch:

diff --git a/src/bhyve/libvirtd_bhyve.aug b/src/bhyve/libvirtd_bhyve.aug
index 66079376c4..9f4e582ab9 100644
--- a/src/bhyve/libvirtd_bhyve.aug
+++ b/src/bhyve/libvirtd_bhyve.aug
@@ -15,7 +15,7 @@ module Libvirtd_bhyve =
let bool_val = store /0|1/
let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
-   let str_array_val = counter "el" . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
+   let str_array_val = counter "el" . array_start . ( ( str_array_element . 
array_sep ) * . str_array_element ? ) ? . array_end
 
let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]
diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug
index 58b9af3707..39049cf139 100644
--- a/src/libxl/libvirtd_libxl.aug
+++ b/src/libxl/libvirtd_libxl.aug
@@ -15,7 +15,7 @@ module Libvirtd_libxl =
let bool_val = store /0|1/
let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
-   let str_array_val = counter "el" . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
+   let str_array_val = counter "el" . array_start . ( ( str_array_element . 
array_sep ) * . str_array_element ? ) ? . array_end
 
let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]
diff --git a/src/locking/virtlockd.aug b/src/locking/virtlockd.aug
index 06d508e6e5..66eb4125ad 100644
--- a/src/locking/virtlockd.aug
+++ b/src/locking/virtlockd.aug
@@ -15,7 +15,7 @@ module Virtlockd =
let bool_val = store /0|1/
let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
-   let str_array_val = counter "el" . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
+   let str_array_val = counter "el" . array_start . ( ( str_array_element . 
array_sep ) * . str_array_element ? ) ? . array_end
 
let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]
diff --git a/src/logging/virtlogd.aug b/src/logging/virtlogd.aug
index 04580734d6..60bbf44305 100644
--- a/src/logging/virtlogd.aug
+++ b/src/logging/virtlogd.aug
@@ -15,7 +15,7 @@ module Virtlogd =
let bool_val = store /0|1/
let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
-   let str_array_val = counter "el" . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
+   let str_array_val = counter "el" . array_start . ( ( str_array_element . 
array_sep ) * . str_array_element ? ) ? . array_end
 
let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]
diff --git a/src/lxc/libvirtd_lxc.aug b/src/lxc/libvirtd_lxc.aug
index be6402cc01..e6ab5f0dde 100644
--- a/src/lxc/libvirtd_lxc.aug
+++ b/src/lxc/libvirtd_lxc.aug
@@ -15,7 +15,7 @@ module Libvirtd_lxc =
let bool_val = store /0|1/
let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
-   let str_array_val = counter "el" . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
+   let str_array_val = counter "el" . array_start . ( ( str_array_element . 
array_sep ) * . str_array_element ? ) ? . array_end
 
let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 404498b611..aceace7d86 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -15,7 +15,7 @@ module Libvirtd_qemu =
let bool_val = store /0|1/
let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
-   let str_array_val = counter "el" . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
+   let str_array_val = counter "el" . array_start . ( ( str_array_element . 
array_sep ) * . str_array_element ? ) ? . array_end
 
let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]

[libvirt PATCH 2/7] po: delete empty translations

[Daniel P . Berrangé]

There is no need to keep .po files which are 100% untranslated in
git. New languages can be added on demand.

Signed-off-by: Daniel P. Berrangé 
---
 po/LINGUAS  | 57 -
 po/af.mini.po   | 19 ---
 po/am.mini.po   | 19 ---
 po/anp.mini.po  | 19 ---
 po/ar.mini.po   | 23 --
 po/ast.mini.po  | 19 ---
 po/bal.mini.po  | 19 ---
 po/be.mini.po   | 20 
 po/bn.mini.po   | 21 -
 po/bo.mini.po   | 19 ---
 po/br.mini.po   | 19 ---
 po/brx.mini.po  | 19 ---
 po/cy.mini.po   | 23 --
 po/de_CH.mini.po| 19 ---
 po/eo.mini.po   | 19 ---
 po/et.mini.po   | 21 -
 po/eu.mini.po   | 21 -
 po/fa.mini.po   | 19 ---
 po/fil.mini.po  | 19 ---
 po/fur.mini.po  | 19 ---
 po/ga.mini.po   | 20 
 po/gl.mini.po   | 21 -
 po/he.mini.po   | 21 -
 po/hr.mini.po   | 20 
 po/ia.mini.po   | 19 ---
 po/ilo.mini.po  | 19 ---
 po/is.mini.po   | 21 -
 po/ka.mini.po   | 21 -
 po/kk.mini.po   | 19 ---
 po/km.mini.po   | 19 ---
 po/kw.mini.po   | 20 
 po/k...@kkcor.mini.po | 20 
 po/k...@uccor.mini.po | 20 
 po/kw_GB.mini.po| 20 
 po/ky.mini.po   | 19 ---
 po/lt.mini.po   | 22 -
 po/lv.mini.po   | 22 -
 po/mai.mini.po  | 19 ---
 po/mn.mini.po   | 19 ---
 po/my.mini.po   | 19 ---
 po/nds.mini.po  | 19 ---
 po/ne.mini.po   | 19 ---
 po/nn.mini.po   | 21 -
 po/nso.mini.po  | 21 -
 po/ro.mini.po   | 22 -
 po/si.mini.po   | 21 -
 po/sk.mini.po   | 21 -
 po/sl.mini.po   | 22 -
 po/sq.mini.po   | 23 --
 po/tg.mini.po   | 19 ---
 po/th.mini.po   | 21 -
 po/tr.mini.po   | 21 -
 po/tw.mini.po   | 19 ---
 po/ur.mini.po   | 21 -
 po/wba.mini.po  | 19 ---
 po/yo.mini.po   | 19 ---
 po/zh_HK.mini.po| 19 ---
 po/zu.mini.po   | 21 -
 58 files changed, 1201 deletions(-)
 delete mode 100644 po/af.mini.po
 delete mode 100644 po/am.mini.po
 delete mode 100644 po/anp.mini.po
 delete mode 100644 po/ar.mini.po
 delete mode 100644 po/ast.mini.po
 delete mode 100644 po/bal.mini.po
 delete mode 100644 po/be.mini.po
 delete mode 100644 po/bn.mini.po
 delete mode 100644 po/bo.mini.po
 delete mode 100644 po/br.mini.po
 delete mode 100644 po/brx.mini.po
 delete mode 100644 po/cy.mini.po
 delete mode 100644 po/de_CH.mini.po
 delete mode 100644 po/eo.mini.po
 delete mode 100644 po/et.mini.po
 delete mode 100644 po/eu.mini.po
 delete mode 100644 po/fa.mini.po
 delete mode 100644 po/fil.mini.po
 delete mode 100644 po/fur.mini.po
 delete mode 100644 po/ga.mini.po
 delete mode 100644 po/gl.mini.po
 delete mode 100644 po/he.mini.po
 delete mode 100644 po/hr.mini.po
 delete mode 100644 po/ia.mini.po
 delete mode 100644 po/ilo.mini.po
 delete mode 100644 po/is.mini.po
 delete mode 100644 po/ka.mini.po
 delete mode 100644 po/kk.mini.po
 delete mode 100644 po/km.mini.po
 delete mode 100644 po/kw.mini.po
 delete mode 100644 po/k...@kkcor.mini.po
 delete mode 100644 po/k...@uccor.mini.po
 delete mode 100644 po/kw_GB.mini.po
 delete mode 100644 po/ky.mini.po
 delete mode 100644 po/lt.mini.po
 delete mode 100644 po/lv.mini.po
 delete mode 100644 po/mai.mini.po
 delete mode 100644 po/mn.mini.po
 delete mode 100644 po/my.mini.po
 delete mode 100644 po/nds.mini.po
 delete mode 100644 po/ne.mini.po
 delete mode 100644 po/nn.mini.po
 delete mode 100644 po/nso.mini.po
 delete mode 100644 po/ro.mini.po
 delete mode 100644 po/si.mini.po
 delete mode 100644 po/sk.mini.po
 delete mode 100644 po/sl.mini.po
 delete mode 100644 po/sq.mini.po
 delete mode 100644 po/tg.mini.po
 delete mode 100644 po/th.mini.po
 delete mode 100644 po/tr.mini.po
 delete mode 100644 po/tw.mini.po
 delete mode 100644 po/ur.mini.po
 delete mode 100644 po/wba.mini.po
 delete mode 100644 po/yo.mini.po
 delete mode 100644 po/zh_HK.mini.po
 delete mode 100644 po/zu.mini.po

diff --git a/po/LINGUAS b/po/LINGUAS
index 2b499d2383..d4cfe63fe2 100644
--- a/po/LINGUAS
+++ b/po/LINGUAS
@@ -1,99 +1,42 @@
-af
-am
-anp
-ar
 as
-ast
-bal
-be
 bg
 bn_IN
-bn
-bo
-br
-brx
 bs
 ca
 cs
-cy
 da
-de_CH
 de
 el
 en_GB
-eo
 es
-et
-eu
-fa
-fil
 fi
 fr
-fur
-ga
-gl
 gu
-he
 hi
-hr
 hu
-ia
 id
-ilo
-is
 

[libvirt PATCH 0/7] po: updates to switch over to use of Weblate

[Daniel P . Berrangé]

The Fedora translation team has stopped using Zanata because the
software itself is dead upstream. In its place is the Weblate
platform. In theory we should have been able to work with Weblate in the
same way as we did for Zanata, pushing a pot file periodically, and
pulling .po files periodically. In practice this fails for libvirt.git
because Weblates RPC API doesn't scale sufficiently well. It will
frequently throw errors with the large libvirt.pot file and it gets
slower at an exponential rate as you add more languages.

Weblate has another mode of operating though which is way more common,
whereby it directly pulls a .pot from your git repo, and then directly
pushes .po files back, either using a trusted SSH key, or by opening a
merge request for GitLab/GitHub/etc.  This is the mode we're going to
have to use in libvirt projects.

Compared to what we're currently doing with Zanata the downsides are:

 - We have to store libvirt.pot in git and refresh it periodically

 - The .po files are only partially minimized, as while they have
   locations stripped, they still contain non-translated msgids

The plussides are

 - We don't have to interact with Weblate at all, only the libvirt
   git repo

 - We'll be able to use the normal meson i18n integration, merely
   by calling

 i18n.gettext(meson.project_name(),
  args: ['--sort-output'],
  preset: 'glib')

I'm intending to open discussion with weblate maintainers to see if
either of those two downsides can be eliminated via feature enhancements
to Weblate. In the meanwhile we just have to accept them, as otherwise
we're not going to get any translations since Zanata is dead.

Daniel P. Berrangé (7):
  po: switch to using LINGUAS file for list of languages
  po: delete empty translations
  po: refresh to drop unused translations
  po: rename the .mini.po files to have just a .po suffix
  po: generate .pot file with strings in alphabetical order
  po: stop stripping non-translated strings from po files
  po: go back to storing the .pot file in git

 Makefile.am  | 1 -
 po/LINGUAS   |42 +
 po/Makefile.am   |50 +-
 po/af.mini.po|19 -
 po/am.mini.po|19 -
 po/anp.mini.po   |19 -
 po/ar.mini.po|23 -
 po/{as.mini.po => as.po} |   402 +-
 po/ast.mini.po   |19 -
 po/bal.mini.po   |19 -
 po/be.mini.po|20 -
 po/{bg.mini.po => bg.po} | 5 +-
 po/bn.mini.po|21 -
 po/{bn_IN.mini.po => bn_IN.po}   |   126 +-
 po/bo.mini.po|19 -
 po/br.mini.po|19 -
 po/brx.mini.po   |19 -
 po/{bs.mini.po => bs.po} | 5 +-
 po/{ca.mini.po => ca.po} |15 +-
 po/{cs.mini.po => cs.po} |   523 +-
 po/cy.mini.po|23 -
 po/{da.mini.po => da.po} | 5 +-
 po/{de.mini.po => de.po} |   408 +-
 po/de_CH.mini.po |19 -
 po/{el.mini.po => el.po} | 2 +-
 po/{en_GB.mini.po => en_GB.po}   |   402 +-
 po/eo.mini.po|19 -
 po/{es.mini.po => es.po} |   409 +-
 po/et.mini.po|21 -
 po/eu.mini.po|21 -
 po/fa.mini.po|19 -
 po/{fi.mini.po => fi.po} |26 +-
 po/fil.mini.po   |19 -
 po/{fr.mini.po => fr.po} |75 +-
 po/fur.mini.po   |19 -
 po/ga.mini.po|20 -
 po/gl.mini.po|21 -
 po/{gu.mini.po => gu.po} |   408 +-
 po/he.mini.po|21 -
 po/{hi.mini.po => hi.po} |   300 +-
 po/hr.mini.po|20 -
 po/{hu.mini.po => hu.po} | 5 +-
 po/ia.mini.po|19 -
 po/{id.mini.po => id.po} | 2 +-
 po/ilo.mini.po   |19 -
 po/is.mini.po|21 -
 po/{it.mini.po => it.po} |   175 +-
 po/{ja.mini.po => ja.po} |   407 +-
 po/ka.mini.po|21 -
 po/kk.mini.po|19 -
 po/km.mini.po|19 -
 po/{kn.mini.po => kn.po} |   407 +-
 po/{ko.mini.po => ko.po} |   240 +-
 po/kw.mini.po|20 -
 po/k...@kkcor.mini.po  |20 -
 po/k...@uccor.mini.po  |20 -
 po/kw_GB.mini.po |20 -
 po/ky.mini.po|19 -
 po/libvirt.pot   | 48582 +
 po/lt.mini.po|22 -

[libvirt PATCH 6/7] po: stop stripping non-translated strings from po files

[Daniel P . Berrangé]

We previously adopted a minimization technique for po files which
stripped source locations and non-translated msgids in order to save
space in the git repos and have saner commit diffs.

At this time it is not possible to integrate with weblate while having
non-translated msgids stripped, as it will immediately add them back
again.

By keeping all non-translated msgids, our .po files are about x2 the
size at 37 MB vs the original 18 MB. This is still way better than the
original po/ directory which was 109 MB. We're saving 38 MB by still
omitting source file locations, and another 34 MB are saved by the
dropping of all languages which are 100% untranslated.

Signed-off-by: Daniel P. Berrangé 
---
 Makefile.am|  1 -
 po/Makefile.am |  4 +---
 scripts/minimize-po.py | 54 --
 3 files changed, 1 insertion(+), 58 deletions(-)
 delete mode 100755 scripts/minimize-po.py

diff --git a/Makefile.am b/Makefile.am
index d56deeb080..ce155bdb87 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -66,7 +66,6 @@ EXTRA_DIST = \
   scripts/header-ifdef.py \
   scripts/hvsupport.py \
   scripts/hyperv_wmi_generator.py \
-  scripts/minimize-po.py \
   scripts/mock-noinline.py \
   scripts/prohibit-duplicate-header.py \
   scripts/reformat-news.py \
diff --git a/po/Makefile.am b/po/Makefile.am
index a31f8ba584..c4c474ccb8 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -48,9 +48,7 @@ update-po: $(POTFILE)
  echo "Minimizing $$lang content" && \
  $(MSGMERGE) --no-location --no-fuzzy-matching --sort-output \
$$lang.po $(POTFILE) | \
- $(SED) $(SED_PO_FIXUP_ARGS) | \
- $(RUNUTF8) $(PYTHON) $(top_srcdir)/scripts/minimize-po.py > \
-   $(srcdir)/$$lang.po-t && \
+ $(SED) $(SED_PO_FIXUP_ARGS) > $(srcdir)/$$lang.po-t && \
  mv $$lang.po-t $$lang.po
done
 
diff --git a/scripts/minimize-po.py b/scripts/minimize-po.py
deleted file mode 100755
index c305229721..00
--- a/scripts/minimize-po.py
+++ /dev/null
@@ -1,54 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (C) 2018-2019 Red Hat, Inc.
-#
-# 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
-# .
-
-import re
-import sys
-
-block = []
-msgstr = False
-empty = False
-unused = False
-fuzzy = False
-
-for line in sys.stdin:
-if line.isspace():
-if not empty and not unused and not fuzzy:
-for b in block:
-print(b, end='')
-
-block = []
-msgstr = False
-fuzzy = False
-block.append(line)
-else:
-if line.startswith("msgstr"):
-msgstr = True
-empty = True
-
-if line[0] == '#' and "fuzzy" in line:
-fuzzy = True
-if line.startswith("#~ msgstr"):
-unused = True
-if msgstr and re.search(r'".+"', line):
-empty = False
-
-block.append(line)
-
-if not empty and not unused and not fuzzy:
-for b in block:
-print(b, end='')
-- 
2.26.2

[libvirt PATCH 1/7] po: switch to using LINGUAS file for list of languages

[Daniel P . Berrangé]

To enable translation management systems to add new languages they need
to be able to modify the supported language list. The LINGUAS file is a
simple standard format that can be used for the language list, as this
is easier than modifying make variables.

Signed-off-by: Daniel P. Berrangé 
---
 po/LINGUAS | 99 ++
 po/Makefile.am | 14 +--
 2 files changed, 101 insertions(+), 12 deletions(-)
 create mode 100644 po/LINGUAS

diff --git a/po/LINGUAS b/po/LINGUAS
new file mode 100644
index 00..2b499d2383
--- /dev/null
+++ b/po/LINGUAS
@@ -0,0 +1,99 @@
+af
+am
+anp
+ar
+as
+ast
+bal
+be
+bg
+bn_IN
+bn
+bo
+br
+brx
+bs
+ca
+cs
+cy
+da
+de_CH
+de
+el
+en_GB
+eo
+es
+et
+eu
+fa
+fil
+fi
+fr
+fur
+ga
+gl
+gu
+he
+hi
+hr
+hu
+ia
+id
+ilo
+is
+it
+ja
+ka
+kk
+km
+kn
+ko
+kw_GB
+kw@kkcor
+kw
+kw@uccor
+ky
+lt
+lv
+mai
+mk
+ml
+mn
+mr
+ms
+my
+nb
+nds
+ne
+nl
+nn
+nso
+or
+pa
+pl
+pt_BR
+pt
+ro
+ru
+si
+sk
+sl
+sq
+sr@latin
+sr
+sv
+ta
+te
+tg
+th
+tr
+tw
+uk
+ur
+vi
+wba
+yo
+zh_CN
+zh_HK
+zh_TW
+zu
diff --git a/po/Makefile.am b/po/Makefile.am
index ac34b0c1aa..ce9338aa94 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -2,18 +2,7 @@ DOMAIN = $(PACKAGE_NAME)
 COPYRIGHT_HOLDER = The Libvirt authors
 MSGID_BUGS_ADDRESS = https://libvirt.org/bugs.html
 
-LANGS := \
-   af am anp ar as ast bal be bg bn_IN \
-   bn bo br brx bs ca cs cy da de_CH \
-   de el en_GB eo es et eu fa fil fi \
-   fr fur ga gl gu he hi hr hu ia \
-   id ilo is it ja ka kk km kn ko \
-   kw_GB kw@kkcor kw kw@uccor ky lt lv mai mk ml \
-   mn mr ms my nb nds ne nl nn nso \
-   or pa pl pt_BR pt ro ru si sk sl \
-   sq sr@latin sr sv ta te tg th tr tw \
-   uk ur vi wba yo zh_CN zh_HK zh_TW zu
-
+LANGS := $(shell cat $(srcdir)/LINGUAS)
 
 POTFILES_IN = $(srcdir)/POTFILES.in
 POTFILES: $(POTFILES_IN)
@@ -29,6 +18,7 @@ GMOFILES := $(LANGS:%=%.gmo)
 CLEANFILES = $(POTFILE) $(POFILES) $(GMOFILES) POTFILES
 
 EXTRA_DIST = \
+   LINGUAS \
$(POTFILES_IN) \
$(POMINIFILES)
 
-- 
2.26.2

[libvirt PATCH 4/7] po: rename the .mini.po files to have just a .po suffix

[Daniel P . Berrangé]

A .mini.po file is exactly the same format as a .po file. We just used
the alternative extension as we wanted to be able to store both full and
minimized forms in the same directory.

This complicates integration with some translation tools, however, which
only really expect to see $LANG.po as a filename.

With this change we drop the rules for creating non-minimized po files,
and thus the po/*.po are always minimized. A useful side effect is that
we no longer run msgmerge during creation of the gmo files, and thus
don't need to have a date override to get reproducible builds.

Signed-off-by: Daniel P. Berrangé 
---
 po/Makefile.am   | 29 +++-
 po/{as.mini.po => as.po} |  0
 po/{bg.mini.po => bg.po} |  0
 po/{bn_IN.mini.po => bn_IN.po}   |  0
 po/{bs.mini.po => bs.po} |  0
 po/{ca.mini.po => ca.po} |  0
 po/{cs.mini.po => cs.po} |  0
 po/{da.mini.po => da.po} |  0
 po/{de.mini.po => de.po} |  0
 po/{el.mini.po => el.po} |  0
 po/{en_GB.mini.po => en_GB.po}   |  0
 po/{es.mini.po => es.po} |  0
 po/{fi.mini.po => fi.po} |  0
 po/{fr.mini.po => fr.po} |  0
 po/{gu.mini.po => gu.po} |  0
 po/{hi.mini.po => hi.po} |  0
 po/{hu.mini.po => hu.po} |  0
 po/{id.mini.po => id.po} |  0
 po/{it.mini.po => it.po} |  0
 po/{ja.mini.po => ja.po} |  0
 po/{kn.mini.po => kn.po} |  0
 po/{ko.mini.po => ko.po} |  0
 po/{mk.mini.po => mk.po} |  0
 po/{ml.mini.po => ml.po} |  0
 po/{mr.mini.po => mr.po} |  0
 po/{ms.mini.po => ms.po} |  0
 po/{nb.mini.po => nb.po} |  0
 po/{nl.mini.po => nl.po} |  0
 po/{or.mini.po => or.po} |  0
 po/{pa.mini.po => pa.po} |  0
 po/{pl.mini.po => pl.po} |  0
 po/{pt.mini.po => pt.po} |  0
 po/{pt_BR.mini.po => pt_BR.po}   |  0
 po/{ru.mini.po => ru.po} |  0
 po/{sr.mini.po => sr.po} |  0
 po/{s...@latin.mini.po => s...@latin.po} |  0
 po/{sv.mini.po => sv.po} |  0
 po/{ta.mini.po => ta.po} |  0
 po/{te.mini.po => te.po} |  0
 po/{uk.mini.po => uk.po} |  0
 po/{vi.mini.po => vi.po} |  0
 po/{zh_CN.mini.po => zh_CN.po}   |  0
 po/{zh_TW.mini.po => zh_TW.po}   |  0
 43 files changed, 7 insertions(+), 22 deletions(-)
 rename po/{as.mini.po => as.po} (100%)
 rename po/{bg.mini.po => bg.po} (100%)
 rename po/{bn_IN.mini.po => bn_IN.po} (100%)
 rename po/{bs.mini.po => bs.po} (100%)
 rename po/{ca.mini.po => ca.po} (100%)
 rename po/{cs.mini.po => cs.po} (100%)
 rename po/{da.mini.po => da.po} (100%)
 rename po/{de.mini.po => de.po} (100%)
 rename po/{el.mini.po => el.po} (100%)
 rename po/{en_GB.mini.po => en_GB.po} (100%)
 rename po/{es.mini.po => es.po} (100%)
 rename po/{fi.mini.po => fi.po} (100%)
 rename po/{fr.mini.po => fr.po} (100%)
 rename po/{gu.mini.po => gu.po} (100%)
 rename po/{hi.mini.po => hi.po} (100%)
 rename po/{hu.mini.po => hu.po} (100%)
 rename po/{id.mini.po => id.po} (100%)
 rename po/{it.mini.po => it.po} (100%)
 rename po/{ja.mini.po => ja.po} (100%)
 rename po/{kn.mini.po => kn.po} (100%)
 rename po/{ko.mini.po => ko.po} (100%)
 rename po/{mk.mini.po => mk.po} (100%)
 rename po/{ml.mini.po => ml.po} (100%)
 rename po/{mr.mini.po => mr.po} (100%)
 rename po/{ms.mini.po => ms.po} (100%)
 rename po/{nb.mini.po => nb.po} (100%)
 rename po/{nl.mini.po => nl.po} (100%)
 rename po/{or.mini.po => or.po} (100%)
 rename po/{pa.mini.po => pa.po} (100%)
 rename po/{pl.mini.po => pl.po} (100%)
 rename po/{pt.mini.po => pt.po} (100%)
 rename po/{pt_BR.mini.po => pt_BR.po} (100%)
 rename po/{ru.mini.po => ru.po} (100%)
 rename po/{sr.mini.po => sr.po} (100%)
 rename po/{s...@latin.mini.po => s...@latin.po} (100%)
 rename po/{sv.mini.po => sv.po} (100%)
 rename po/{ta.mini.po => ta.po} (100%)
 rename po/{te.mini.po => te.po} (100%)
 rename po/{uk.mini.po => uk.po} (100%)
 rename po/{vi.mini.po => vi.po} (100%)
 rename po/{zh_CN.mini.po => zh_CN.po} (100%)
 rename po/{zh_TW.mini.po => zh_TW.po} (100%)

diff --git a/po/Makefile.am b/po/Makefile.am
index ce9338aa94..224f16e993 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -11,16 +11,15 @@ POTFILES: $(POTFILES_IN)
$(SED) 's|[@]BUILDDIR[@]|$(top_builddir)|' > $@
 POTFILE_DEPS = $(shell cat POTFILES)
 POTFILE := $(DOMAIN).pot
-POMINIFILES := $(LANGS:%=%.mini.po)
 POFILES := $(LANGS:%=%.po)
 GMOFILES := $(LANGS:%=%.gmo)
 
-CLEANFILES = $(POTFILE) $(POFILES) $(GMOFILES) POTFILES
+CLEANFILES = $(POTFILE) $(GMOFILES) POTFILES
 
 EXTRA_DIST = \
LINGUAS \
$(POTFILES_IN) \
-   $(POMINIFILES)
+   $(POFILES)
 
 if HAVE_GNU_GETTEXT_TOOLS
 
@@ -35,33 +34,23 @@ XGETTEXT_ARGS = \

[libvirt PATCH 5/7] po: generate .pot file with strings in alphabetical order

[Daniel P . Berrangé]

The .po files are stored with strings in alphabetical order instead of
source file location order, because this minimizes the diffs created
when code moves around within or between files.

By default msgmerge will honour the order of strings in the .pot file
when creating a .po file, so it is useful if we also create the .pot
file with desired ordering.

Signed-off-by: Daniel P. Berrangé 
---
 po/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/po/Makefile.am b/po/Makefile.am
index 224f16e993..a31f8ba584 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -32,6 +32,7 @@ XGETTEXT_ARGS = \
--package-name="$(PACKAGE_NAME)" \
--package-version="$(PACKAGE_VERSION)" \
--msgid-bugs-address="$(MSGID_BUGS_ADDRESS)" \
+   --sort-output \
$(NULL)
 
 SED_PO_FIXUP_ARGS = \
-- 
2.26.2

Re: [PATCH v2] README.rst: Replace the installation instructions with link to compiling.html.

[Michal Privoznik]

On 5/18/20 1:59 PM, Yi Wang wrote:

From: Liao Pingfang 

Remove the installation instructions from README.rst, and
add the link to compiling.html.

Signed-off-by: Liao Pingfang 
---
Changes in v2: use compiling.html replace the installation instructions

  README.rst | 26 ++
  1 file changed, 2 insertions(+), 24 deletions(-)


Reviewed-by: Michal Privoznik 

and pushed. Congratulations on your first libvirt contribution!

Michal

[libvirt PATCH] qemu: conf: fix stray comma

[Ján Tomko]

The qemu.conf change broke our augeas test:

qemu/test_libvirtd_qemu.aug:96.3-203.1:exception thrown in test
qemu/test_libvirtd_qemu.aug:96.8-.34:exception: Iterated lens matched less than 
it should
Lens: ../../src/qemu/libvirtd_qemu.aug:170.13-.43:
  Last match: ../../src/qemu/libvirtd_qemu.aug:18.52-.113:
  Not matching: ../../src/qemu/libvirtd_qemu.aug:12.19-.31:
Error encountered at 48:27 (1615 characters into string)
<\n"/dev/ptmx", "/dev/kvm"|=|,\n]\nsave_image_format = "raw>

Fixes: ab5ba57012e9e6ab4f55afdeecd1813dd3ca916b
Signed-off-by: Ján Tomko 
---
Pushed as a trivial build fix

 src/qemu/qemu.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index d7a3f40e78..404961c53e 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -494,7 +494,7 @@
 #cgroup_device_acl = [
 #"/dev/null", "/dev/full", "/dev/zero",
 #"/dev/random", "/dev/urandom",
-#"/dev/ptmx", "/dev/kvm",
+#"/dev/ptmx", "/dev/kvm"
 #]
 #
 # RDMA migration requires the following extra files to be added to the list:
-- 
2.25.4

Re: [PATCH] qemu: do not allow /dev/rtc or /dev/hpet access via the devices cgroup

[Michal Privoznik]

On 5/19/20 10:55 AM, Daniel P. Berrangé wrote:

On Tue, May 19, 2020 at 10:10:54AM +0200, Michal Privoznik wrote:

On 5/19/20 1:06 AM, Paolo Bonzini wrote:

The RTC and HPET modes for the QEMU emulation tick have been dropped almost 9 
years
ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e.  Do not allow them in 
the


qemu.git $ git describe --tags 25f3151ece1d5881826232bebccc21b588d4e03e
v0.14.0-rc0-1169-g25f3151ece

and the minimum supported version is 1.5.0 so this is safe to merge from min
version POV.


devices cgroup policy.

Signed-off-by: Paolo Bonzini 
---
   docs/drvqemu.html.in   | 1 -
   src/qemu/qemu.conf | 1 -
   src/qemu/qemu_cgroup.c | 1 -
   src/qemu/test_libvirtd_qemu.aug.in | 2 --
   4 files changed, 5 deletions(-)


It's not only QEMU that might use these but also a library that is linking
with. However, quick strace showed no access to either of the files so:

Reviewed-by: Michal Privoznik 

And pushed.


This broke make check

https://ci.centos.org/view/libvirt/job/libvirt-check/systems=libvirt-fedora-32/1170/console

though I don't understand why as it looks like it removed all the
right pieces. I wonder if we had a bad dependancy in make rules
meaning we didn't regenerate


Ah, could it be because of the stray comma? From qemu.conf:

#cgroup_device_acl = [
#"/dev/null", "/dev/full", "/dev/zero",
#"/dev/random", "/dev/urandom",
#"/dev/ptmx", "/dev/kvm",
#]

Let me check if removing the comma after /dev/kvm fixes the build.

Michal

Re: [PATCH] qemu: do not allow /dev/rtc or /dev/hpet access via the devices cgroup

[Daniel P . Berrangé]

On Tue, May 19, 2020 at 10:10:54AM +0200, Michal Privoznik wrote:
> On 5/19/20 1:06 AM, Paolo Bonzini wrote:
> > The RTC and HPET modes for the QEMU emulation tick have been dropped almost 
> > 9 years
> > ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e.  Do not allow them 
> > in the
> 
> qemu.git $ git describe --tags 25f3151ece1d5881826232bebccc21b588d4e03e
> v0.14.0-rc0-1169-g25f3151ece
> 
> and the minimum supported version is 1.5.0 so this is safe to merge from min
> version POV.
> 
> > devices cgroup policy.
> > 
> > Signed-off-by: Paolo Bonzini 
> > ---
> >   docs/drvqemu.html.in   | 1 -
> >   src/qemu/qemu.conf | 1 -
> >   src/qemu/qemu_cgroup.c | 1 -
> >   src/qemu/test_libvirtd_qemu.aug.in | 2 --
> >   4 files changed, 5 deletions(-)
> 
> It's not only QEMU that might use these but also a library that is linking
> with. However, quick strace showed no access to either of the files so:
> 
> Reviewed-by: Michal Privoznik 
> 
> And pushed.

This broke make check

https://ci.centos.org/view/libvirt/job/libvirt-check/systems=libvirt-fedora-32/1170/console

though I don't understand why as it looks like it removed all the
right pieces. I wonder if we had a bad dependancy in make rules
meaning we didn't regenerate

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 5/6] tools: secure guest check for AMD in virt-host-validate

[Boris Fiuczynski]

On 5/19/20 8:25 AM, Erik Skultety wrote:

...

diff --git a/docs/kbase/launch_security_sev.rst 
b/docs/kbase/launch_security_sev.rst
index fa602c7432..45166b3886 100644
--- a/docs/kbase/launch_security_sev.rst
+++ b/docs/kbase/launch_security_sev.rst
@@ -30,8 +30,11 @@ Enabling SEV on the host
   
   Before VMs can make use of the SEV feature you need to make sure your
-AMD CPU does support SEV. You can check whether SEV is among the CPU
-flags with:
+AMD CPU does support SEV. You can run ``libvirt-host-validate``
+(libvirt >= 6.4.0) to check if your host supports secure guests or you
+can follow the manual checks below.
+
+You can manually check whether SEV is among the CPU flags with:


^this change should go along the (<6.4.0) in one of the earlier patches into a
standalone patch.


Actually the earlier patches fix the stale cap cache and this update is
because of a new support in libvirt-host-validate. I am not sure that we
should tie these to into one patch.
I would prefer to keep the two doc changes separate and with the changes
that caused the update.


I won't argue against that logic. However, both patch 3 and this one update the
same knowledge article. What IMO matters here the most is that once all of the
changes you're introducing are applied as a unit, the article needs to
reflect both the changes. From that perspective, at least to me it makes total
sense to group the docs changes from both 3/6 and this patch to a single update
to the SEV article accordingly.


Fine, I will sort out the changes and create a single AMD doc update patch.

--
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] qemu: do not allow /dev/rtc or /dev/hpet access via the devices cgroup

[Michal Privoznik]

On 5/19/20 1:06 AM, Paolo Bonzini wrote:

The RTC and HPET modes for the QEMU emulation tick have been dropped almost 9 
years
ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e.  Do not allow them in 
the


qemu.git $ git describe --tags 25f3151ece1d5881826232bebccc21b588d4e03e
v0.14.0-rc0-1169-g25f3151ece

and the minimum supported version is 1.5.0 so this is safe to merge from 
min version POV.



devices cgroup policy.

Signed-off-by: Paolo Bonzini 
---
  docs/drvqemu.html.in   | 1 -
  src/qemu/qemu.conf | 1 -
  src/qemu/qemu_cgroup.c | 1 -
  src/qemu/test_libvirtd_qemu.aug.in | 2 --
  4 files changed, 5 deletions(-)


It's not only QEMU that might use these but also a library that is 
linking with. However, quick strace showed no access to either of the 
files so:


Reviewed-by: Michal Privoznik 

And pushed.

Michal

Re: [libvirt PATCH 3/6] nodedev: refactor nodeDeviceFindNewDevice()

[Erik Skultety]

On Mon, May 11, 2020 at 03:27:58PM +0200, Michal Privoznik wrote:
> On 4/30/20 11:42 PM, Jonathon Jongsma wrote:
> > In preparation for creating mediated devices in libvirt, we will need to
> > wait for new mediated devices to be created as well. Refactor
> > nodeDeviceFindNewDevice() so that we can re-use the main logic from this
> > function to wait for different device types by passing a different
> > 'find' function.
> > 
> > Signed-off-by: Jonathon Jongsma 
> > ---
> >   src/node_device/node_device_driver.c | 33 +++-
> >   1 file changed, 28 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/node_device/node_device_driver.c 
> > b/src/node_device/node_device_driver.c
> > index 6a96a77d92..f948dfbf69 100644
> > --- a/src/node_device/node_device_driver.c
> > +++ b/src/node_device/node_device_driver.c
> > @@ -446,7 +446,8 @@ nodeDeviceGetTime(time_t *t)
> >   return ret;
> >   }
> > -
> > +typedef virNodeDevicePtr (*nodeDeviceFindNewDeviceFunc)(virConnectPtr conn,
> > +const void* 
> > opaque);
> 
> Again, some spaces here, and especially below.
> 
> >   /* When large numbers of devices are present on the host, it's
> >* possible for udev not to realize that it has work to do before we
> >* get here.  We thus keep trying to find the new device we just
> > @@ -462,8 +463,8 @@ nodeDeviceGetTime(time_t *t)
> >*/
> >   static virNodeDevicePtr
> >   nodeDeviceFindNewDevice(virConnectPtr conn,
> > -const char *wwnn,
> > -const char *wwpn)
> > +nodeDeviceFindNewDeviceFunc func,
> > +const void *opaque)
> >   {
> >   virNodeDevicePtr device = NULL;
> >   time_t start = 0, now = 0;
> > @@ -474,7 +475,7 @@ nodeDeviceFindNewDevice(virConnectPtr conn,
> >   virWaitForDevices();
> > -device = nodeDeviceLookupSCSIHostByWWN(conn, wwnn, wwpn, 0);
> > +device = func(conn, opaque);
> >   if (device != NULL)
> >   break;
> > @@ -487,6 +488,28 @@ nodeDeviceFindNewDevice(virConnectPtr conn,
> >   return device;
> >   }
> > +typedef struct _NewSCISHostFuncData
> 
> s/SCIS/SCSI/
> 
> > +{
> > +const char *wwnn;
> > +const char *wwpn;
> > +} NewSCSIHostFuncData;
> 
> We tend to like this split:
> 
> typedef struct _someStruct someStruct;
> struct _someStruct {
>   const char *member;
> };
> 
> Honestly, I don't understand why, I guess it's just a coding style.

The typedef doesn't really bring any benefit in this case. I don't have any
tangible statistics for this, just a plain git grep, but it looks like that the
prevailing style for such private structures actually *is* without any
typedefing which also makes sense.

No further comments on this patch from my side.

-- 
Erik Skultety

Re: [libvirt PATCH 1/6] nodedev: factor out nodeDeviceHasCapability()

[Erik Skultety]

On Mon, May 11, 2020 at 03:28:03PM +0200, Michal Privoznik wrote:
> On 4/30/20 11:42 PM, Jonathon Jongsma wrote:
> > Currently nodeDeviceCreateXML() and nodeDeviceDestroy() only support
> > NPIV HBAs, but we want to be able to create mdev devices as well. This
> > is a first step to enabling that support.
> > 
> > Signed-off-by: Jonathon Jongsma 
> > ---
> >   src/node_device/node_device_driver.c | 90 ++--
> >   1 file changed, 57 insertions(+), 33 deletions(-)
> > 
> > diff --git a/src/node_device/node_device_driver.c 
> > b/src/node_device/node_device_driver.c
> > index ee175e1095..6a96a77d92 100644
> > --- a/src/node_device/node_device_driver.c
> > +++ b/src/node_device/node_device_driver.c
> > @@ -487,6 +487,20 @@ nodeDeviceFindNewDevice(virConnectPtr conn,
> >   return device;
> >   }
> > +static bool
> > +nodeDeviceHasCapability(virNodeDeviceDefPtr def, virNodeDevCapType type)
> > +{
> > +virNodeDevCapsDefPtr cap = NULL;
> > +
> > +cap = def->caps;
> 
> This variable can be initialized to def->caps directly. No need to go
> through NULL, IMO.
> 
> > +while (cap != NULL) {
> > +if (cap->data.type == type)
> > +return true;
> > +cap = cap->next;
> > +}
> > +
> > +return false;
> > +}
> 
> 
> Also, here and for the rest of the patchset - the file uses two spaces
> between functions. Please keep that.
> 
> Michal
> 

With the suggested changes:
Reviewed-by: Erik Skultety 

Re: [PATCH 5/6] tools: secure guest check for AMD in virt-host-validate

[Erik Skultety]

...
> > > diff --git a/docs/kbase/launch_security_sev.rst 
> > > b/docs/kbase/launch_security_sev.rst
> > > index fa602c7432..45166b3886 100644
> > > --- a/docs/kbase/launch_security_sev.rst
> > > +++ b/docs/kbase/launch_security_sev.rst
> > > @@ -30,8 +30,11 @@ Enabling SEV on the host
> > >   
> > >   Before VMs can make use of the SEV feature you need to make sure your
> > > -AMD CPU does support SEV. You can check whether SEV is among the CPU
> > > -flags with:
> > > +AMD CPU does support SEV. You can run ``libvirt-host-validate``
> > > +(libvirt >= 6.4.0) to check if your host supports secure guests or you
> > > +can follow the manual checks below.
> > > +
> > > +You can manually check whether SEV is among the CPU flags with:
> > 
> > ^this change should go along the (<6.4.0) in one of the earlier patches 
> > into a
> > standalone patch.
> 
> Actually the earlier patches fix the stale cap cache and this update is
> because of a new support in libvirt-host-validate. I am not sure that we
> should tie these to into one patch.
> I would prefer to keep the two doc changes separate and with the changes
> that caused the update.

I won't argue against that logic. However, both patch 3 and this one update the
same knowledge article. What IMO matters here the most is that once all of the
changes you're introducing are applied as a unit, the article needs to
reflect both the changes. From that perspective, at least to me it makes total
sense to group the docs changes from both 3/6 and this patch to a single update
to the SEV article accordingly.

-- 
Erik Skultety

Re: [PATCH 4/6] tools: secure guest check on s390 in virt-host-validate

[Erik Skultety]

...

> > > +
> > > +virHostMsgCheck(hvname, "%s", _("for secure guest support"));
> > > +if (ARCH_IS_S390(arch)) {
> > 
> > We don't need ^this check here, facility 158 won't be available on x86.
> 
> Since it is very true that the facility 158 won't be available on x86 you do
> not want to get the message "Hardware or firmware does not provide support
> for IBM Secure Execution" on x86.
> On the other hand you can be on s390 and not have the facility available and
> want to know see the message.

Oops, I missed the last nested else block, please ignore what I said then.

-- 
Erik Skultety