[PATCH] tests: libxl: Mock xs_open and xs_close

2021-05-17 Thread Jim Fehlig
The Xen-related unit tests are failing against the recently released
Xen 4.15. Xen commit 90c9f9f4dd changed the implementation of
libxl_ctx_alloc to use xs_open instead of xs_daemon_open. libvirt has
already mocked xs_daemon-{open,close} and others to allow using libxl
in confined build environments. This patch adds xs_{open,close} to the
list of functions mocked in libxlmock.c

https://github.com/xen-project/xen/commit/90c9f9f4ddd55e11be0506bff10c6237507c6e0d

Signed-off-by: Jim Fehlig 
---
 tests/libxlmock.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/libxlmock.c b/tests/libxlmock.c
index 604dc4bfbe..a52d4bc2ed 100644
--- a/tests/libxlmock.c
+++ b/tests/libxlmock.c
@@ -39,6 +39,14 @@ VIR_MOCK_IMPL_RET_VOID(xs_daemon_open,
 return (void*)0x1;
 }
 
+VIR_MOCK_IMPL_RET_ARGS(xs_open,
+   struct xs_handle *,
+   unsigned long, flags)
+{
+VIR_MOCK_REAL_INIT(xs_open);
+return (void*)0x1;
+}
+
 VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
xc_interface *,
xentoollog_logger *, logger,
@@ -94,6 +102,9 @@ VIR_MOCK_STUB_RET_ARGS(xc_sharing_used_frames,
 VIR_MOCK_STUB_VOID_ARGS(xs_daemon_close,
 struct xs_handle *, handle)
 
+VIR_MOCK_STUB_VOID_ARGS(xs_close,
+struct xs_handle *, xsh)
+
 VIR_MOCK_STUB_RET_ARGS(bind,
int, 0,
int, sockfd,
-- 
2.31.1




[PATCH] tests: Replace deprecated ASN1 code

2021-05-17 Thread Luke Yue
This fixes compiler warnings when building with libtasn1 4.17.0.

Signed-off-by: Luke Yue 
---
 tests/pkix_asn1_tab.c|  2 +-
 tests/virnettlshelpers.c | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/pkix_asn1_tab.c b/tests/pkix_asn1_tab.c
index 5d5ca3db5d..a28d5f20c3 100644
--- a/tests/pkix_asn1_tab.c
+++ b/tests/pkix_asn1_tab.c
@@ -5,7 +5,7 @@
 #include 
 #include 
 
-const ASN1_ARRAY_TYPE pkix_asn1_tab[] = {
+const asn1_static_node pkix_asn1_tab[] = {
   { "PKIX1", 536875024, NULL },
   { NULL, 1073741836, NULL },
   { "id-pkix", 1879048204, NULL },
diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c
index ce38571b0a..905e633e60 100644
--- a/tests/virnettlshelpers.c
+++ b/tests/virnettlshelpers.c
@@ -37,8 +37,8 @@ VIR_LOG_INIT("tests.nettlshelpers");
  * These store some static data that is needed when
  * encoding extensions in the x509 certs
  */
-ASN1_TYPE pkix_asn1;
-extern const ASN1_ARRAY_TYPE pkix_asn1_tab[];
+asn1_node pkix_asn1;
+extern const asn1_static_node pkix_asn1_tab[];
 
 /*
  * To avoid consuming random entropy to generate keys,
@@ -107,7 +107,7 @@ void testTLSCleanup(const char *keyfile)
 /*
  * Turns an ASN1 object into a DER encoded byte array
  */
-static void testTLSDerEncode(ASN1_TYPE src,
+static void testTLSDerEncode(asn1_node src,
  const char *src_name,
  gnutls_datum_t * res)
 {
@@ -267,7 +267,7 @@ testTLSGenerateCert(struct testTLSCertReq *req,
  * the 'critical' field which we want control over
  */
 if (req->basicConstraintsEnable) {
-ASN1_TYPE ext = ASN1_TYPE_EMPTY;
+asn1_node ext = NULL;
 
 asn1_create_element(pkix_asn1, "PKIX1.BasicConstraints", );
 asn1_write_value(ext, "cA", req->basicConstraintsIsCA ? "TRUE" : 
"FALSE", 1);
@@ -292,7 +292,7 @@ testTLSGenerateCert(struct testTLSCertReq *req,
  * to be 'critical'
  */
 if (req->keyUsageEnable) {
-ASN1_TYPE ext = ASN1_TYPE_EMPTY;
+asn1_node ext = NULL;
 char str[2];
 
 str[0] = req->keyUsageValue & 0xff;
@@ -321,7 +321,7 @@ testTLSGenerateCert(struct testTLSCertReq *req,
  * set this the hard way building up ASN1 data ourselves
  */
 if (req->keyPurposeEnable) {
-ASN1_TYPE ext = ASN1_TYPE_EMPTY;
+asn1_node ext = NULL;
 
 asn1_create_element(pkix_asn1, "PKIX1.ExtKeyUsageSyntax", );
 if (req->keyPurposeOID1) {
-- 
2.31.1



Qemu block filter insertion/removal API

2021-05-17 Thread Vladimir Sementsov-Ogievskiy

Hi all!

I'd like to be sure that we know where we are going to.

In blockdev-era where qemu user is aware about block nodes, all nodes have good 
names and controlled by user we can efficiently use block filters.

We already have some useful filters: copy-on-read, throttling, compress. In my 
parallel series I make backup-top filter public and useful without backup block 
jobs. But now filters could be inserted only together with opening their child. 
We can specify filters in qemu cmdline, or filter can take place in the block 
node chain created by blockdev-add.

Still, it would be good to insert/remove filters on demand.

Currently we are going to use x-blockdev-reopen for this. Still it can't be used to 
insert a filter above root node (as x-blockdev-reopen can change only block node options 
and their children). In my series "[PATCH 00/21] block: publish backup-top 
filter" I propose (as Kevin suggested) to modify qom-set, so that it can set drive 
option of running device. That's not difficult, but it means that we have different 
scenario of inserting/removing filters:

1. filter above root node X:

inserting:

  - do blockdev-add to add a filter (and specify X as its child)
  - do qom-set to set new filter as a rood node instead of X

removing

  - do qom-set to make X a root node again
  - do blockdev-del to drop a filter

2. filter between two block nodes P and X. (For example, X is a backing child 
of P)

inserting

  - do blockdev-add to add a filter (and specify X as its child)
  - do blockdev-reopen to set P.backing = filter

remvoing

  - do blockdev-reopen to set P.backing = X
  - do blockdev-del to drop a filter


And, probably we'll want transaction support for all these things.


Is it OK? Or do we need some kind of additional blockdev-replace command, that 
can replace one node by another, so in both cases we will do

inserting:
 
  - blockdev-add filter

  - blockdev-replace (make all parents of X to point to the new filter instead 
(except for the filter itself of course)

removing
  
  - blockdev-replace (make all parante of filter to be parents of X instead)

  - blockdev-del filter


It's simple to implement, and it seems for me that it is simpler to use. Any 
thoughts?

--
Best regards,
Vladimir



Re: [PATCH] viridentity: Fix ref/unref imbalance in VIR_IDENTITY_AUTORESTORE

2021-05-17 Thread Daniel P . Berrangé
On Mon, May 17, 2021 at 06:12:56PM +0200, Michal Privoznik wrote:
> The basic use case of VIR_IDENTITY_AUTORESTORE() is in
> conjunction with virIdentityElevateCurrent(). What happens is
> that virIdentityElevateCurrent() gets current identity (which
> increases the refcounter of thread local virIdentity object) and
> returns a pointer to it. Later, when the variable goes out of
> scope the virIdentityRestoreHelper() is called which calls
> virIdentitySetCurrent() over the old identity. But this means
> that the refcounter is increased again.
> 
> Therefore, we have to explicitly decrease the refcounter by
> calling g_object_unref().

Opps, yes, i remember thinking about this and clearly forgot
it again.

> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> I've observed this imbalance whilst running qemuxml2argvtest under
> valgrind:
> 
> ==10412== 949 (40 direct, 909 indirect) bytes in 1 blocks are definitely lost 
> in loss record 524 of 539
> ==10412==at 0x48397EF: malloc (vg_replace_malloc.c:307)
> ==10412==by 0x50806F8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.7)
> ==10412==by 0x50992FD: g_slice_alloc (in 
> /usr/lib64/libglib-2.0.so.0.6600.7)
> ==10412==by 0x50999B9: g_slice_alloc0 (in 
> /usr/lib64/libglib-2.0.so.0.6600.7)
> ==10412==by 0x518D9AB: g_type_create_instance (in 
> /usr/lib64/libgobject-2.0.so.0.6600.7)
> ==10412==by 0x51739DC: g_object_new_internal (in 
> /usr/lib64/libgobject-2.0.so.0.6600.7)
> ==10412==by 0x5174F5C: g_object_new_with_properties (in 
> /usr/lib64/libgobject-2.0.so.0.6600.7)
> ==10412==by 0x5175978: g_object_new (in 
> /usr/lib64/libgobject-2.0.so.0.6600.7)
> ==10412==by 0x496C3C6: virIdentityNew (viridentity.c:407)
> ==10412==by 0x496BF1A: virIdentityGetSystem (viridentity.c:318)
> ==10412==by 0x117CEF: testCompareXMLToArgv (qemuxml2argvtest.c:653)
> ==10412==by 0x148505: virTestRun (testutils.c:142)
> 
> Test #315 doesn't tickle the bug, while test #316 does.
> 
>  src/util/viridentity.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 


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



[PATCH] viridentity: Fix ref/unref imbalance in VIR_IDENTITY_AUTORESTORE

2021-05-17 Thread Michal Privoznik
The basic use case of VIR_IDENTITY_AUTORESTORE() is in
conjunction with virIdentityElevateCurrent(). What happens is
that virIdentityElevateCurrent() gets current identity (which
increases the refcounter of thread local virIdentity object) and
returns a pointer to it. Later, when the variable goes out of
scope the virIdentityRestoreHelper() is called which calls
virIdentitySetCurrent() over the old identity. But this means
that the refcounter is increased again.

Therefore, we have to explicitly decrease the refcounter by
calling g_object_unref().

Signed-off-by: Michal Privoznik 
---

I've observed this imbalance whilst running qemuxml2argvtest under
valgrind:

==10412== 949 (40 direct, 909 indirect) bytes in 1 blocks are definitely lost 
in loss record 524 of 539
==10412==at 0x48397EF: malloc (vg_replace_malloc.c:307)
==10412==by 0x50806F8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.7)
==10412==by 0x50992FD: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.7)
==10412==by 0x50999B9: g_slice_alloc0 (in 
/usr/lib64/libglib-2.0.so.0.6600.7)
==10412==by 0x518D9AB: g_type_create_instance (in 
/usr/lib64/libgobject-2.0.so.0.6600.7)
==10412==by 0x51739DC: g_object_new_internal (in 
/usr/lib64/libgobject-2.0.so.0.6600.7)
==10412==by 0x5174F5C: g_object_new_with_properties (in 
/usr/lib64/libgobject-2.0.so.0.6600.7)
==10412==by 0x5175978: g_object_new (in 
/usr/lib64/libgobject-2.0.so.0.6600.7)
==10412==by 0x496C3C6: virIdentityNew (viridentity.c:407)
==10412==by 0x496BF1A: virIdentityGetSystem (viridentity.c:318)
==10412==by 0x117CEF: testCompareXMLToArgv (qemuxml2argvtest.c:653)
==10412==by 0x148505: virTestRun (testutils.c:142)

Test #315 doesn't tickle the bug, while test #316 does.

 src/util/viridentity.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index e7e5c31241..eb77f69e2e 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -197,8 +197,12 @@ void virIdentityRestoreHelper(virIdentity **identptr)
 {
 virIdentity *ident = *identptr;
 
-if (ident != NULL)
+if (ident != NULL) {
 virIdentitySetCurrent(ident);
+/* virIdentitySetCurrent() grabs its own reference.
+ * We don't need ours anymore. */
+g_object_unref(ident);
+}
 }
 
 #define TOKEN_BYTES 16
-- 
2.26.3



RFC: Qemu backup interface plans

2021-05-17 Thread Vladimir Sementsov-Ogievskiy

Hi all!

I'd like to share and discuss some plans on Qemu backup interface I have. 
(actually, most of this I've presented on KVM Forum long ago.. But now I'm a 
lot closer to realization:)


I'd start with a reminder about image fleecing:

We have image fleecing scheme to export point-in-time state of active
disk (iotest 222):


  backup(sync=none)
 ┌───┐
 ▼   │
┌┐ ┌┐  backing ┌─┐
│ NBD export │ ─── │ temp qcow2 img │ ───▶ │ active disk │
└┘ └┘  └─┘
 ▲
┌┐   │
│ guest blk  │ ──┘
└┘


Actually, backup job inserts a backup-top filter, so in detail it looks
like:

  backup(sync=none)
 ┌───┐
 ▼   │
┌┐ ┌┐  backing ┌─┐
│ NBD export │ ─── │ temp qcow2 img │ ───▶ │ active disk │
└┘ └┘  └─┘
 ▲   ▲
 │ target│
 │   │
┌┐ ┌┐  backing   │
│ guest blk  │ ──▶ │   backup-top   │ ───┘
└┘ └┘
 


This scheme is also called external backup or pull backup. It allows some 
external tool to write data to actual backup, and Qemu only provides this data.

We support also incremental external backup: Qemu can manage dirty bitmaps in any way 
user wants, and we can export bitmaps through NBD protocol. So, client of NBD export can 
get the bitmap, and read only "dirty" regions of exported image.

What we lack in this scheme:

1. handling dirty bitmap in backup-top filter: backup-top does copy-before-write 
operation on any guest write, when actually we are interested only in "dirty" 
regions for incremental backup

Probable solution would allowing specifying bitmap for sync=none mode of 
backup, but I think what I propose below is better.

2. [actually it's a tricky part of 1]: possibility to not do copy-before-write 
operations for regions that was already copied to final backup. With normal 
Qemu backup job, this is achieved by the fact that block-copy state with its 
internal bitmap is shared between backup job and copy-before-write filter.

3. Not a real problem but fact: backup block-job does nothing in the scheme, 
the whole job is done by filter. So, it would be interesting to have a 
possibility to simply insert/remove the filter, and avoid block-job creation 
and managing at all for external backup. (and I'd like to send another RFC on 
how to insert/remove filters, let's not discuss it here).


Next. Think about internal backup. It has one drawback too:
4. If target is remote with slow connection, copy-before-write operations will 
slow down guest writes appreciably.

It may be solved with help of image fleecing: we create temporary qcow2 image, 
setup fleecing scheme, and instead of exporting temp image through NBD we start 
a second backup with source = temporary image and target would be real backup 
target (NBD for example). Still, with such solution there are same [1,2] 
problems, 3 becomes worse:

5. We'll have two jobs and two automatically inserted filters, when actually 
one filter and one job are enough (as first job is needed only to insert a 
filter, second job doesn't need a filter at all).

Note also, that this (starting two backup jobs to make push backup with 
fleecing) doesn't work now, op-blockers will be against. It's simple to fix 
(and in Virtuozzo we live with downstream-only patch, which allows push backup 
with fleecing, based on starting two backup jobs).. But I never send a patch, 
as I have better plan, which will solve all listed problems.


So, what I propose:

1. We make backup-top filter public, so that it could be inserted/removed where 
user wants through QMP (how to properly insert/remove filter I'll post another 
RFC, as backup-top is not the only filter that can be usefully inserted 
somewhere). For this first step I've sent a series today:

  subject: [PATCH 00/21] block: publish backup-top filter
  id: <20210517064428.16223-1-vsement...@virtuozzo.com>
  patchew: 
https://patchew.org/QEMU/20210517064428.16223-1-vsement...@virtuozzo.com/

(note, that one of things in this series is rename 
s/backup-top/copy-before-write/, still, I call it backup-top in this letter)

This solves [3]. [4, 5] are solved 

Re: [PATCH libvirt v1] tests: add capabilities for QEMU 6.0.0 on s390x

2021-05-17 Thread Shalini Chellathurai Saroja



On 5/13/21 12:45 PM, Michal Prívozník wrote:

On 5/13/21 11:53 AM, Andrea Bolognani wrote:

On Thu, May 13, 2021 at 11:39:24AM +0200, Michal Prívozník wrote:

On 5/12/21 6:11 PM, Andrea Bolognani wrote:

Overall looks reasonable, but comparing the computed capabilities
with those for QEMU 5.2 highlights a couple of changes that I'm not
so sure about, specifically

   --- tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml   2021-05-12
10:52:29.826021415 +0200
   +++ tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml   2021-05-12
17:39:47.778445212 +0200
   @@ -87,7 +87,6 @@
  
  
  
   -  
   @@ -115,7 +114,6 @@
  
  
  
   -  

Because of the former I would expect the s390x-ccw-graphics xml2argv
test to fail, but it looks like we don't do a good job at validating
the  element and that QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW in
particular is completely unused.

As for the latter there doesn't seem to be any test coverage outside
of x86_64, so perhaps it would be a good idea to look into that?

pmem was reported by QEMU even if it was later denied. This was fixed by:

https://gitlab.com/qemu-project/qemu/-/commit/def835f0da0d153b397071e6bb8f2b46f51f96b4

qemu.git $ git describe --contains def835f0da0d153b397071e6bb8f2b46f51f96b4
v6.0.0-rc0~77^2

Okay for the pmem part - it was reported as available on s390x even
though it probably never worked.


And it's the same story with virtio-gpu-ccw:

https://gitlab.com/qemu-project/qemu/-/commit/adcf33a504de29feb720736051dc32889314c9e6

qemu.git $ git describe --contains adcf33a504de29feb720736051dc32889314c9e6
v6.0.0-rc1~5^2~1

But virtio-gpu-ccw surely should be reported as available on s390x?
Perhaps I'm missing something obvious in the commit you pointed me
to and I should grab another coffee :)


I admit, I don't know. I thought that virtio-gpu was also misleadingly
reported as supported. But as you pointed out it's not the case.
Shalini, can you chime in?


Hello Michal, Andrea,

Thank you for the review. I am working on resolving the "virtio-gpu-ccw" 
availability.




Michal


--
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294




Re: [PATCH 4/4] virCapabilitiesHostNUMAFormat: Bring variables into loops

2021-05-17 Thread Michal Prívozník
On 5/17/21 3:46 PM, Ján Tomko wrote:
> On a Monday in 2021, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik 
>> ---
>> src/conf/capabilities.c | 9 +
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index 1dae6d38cc..915cd3149e 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -807,8 +807,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
>>   virCapsHostNUMA *caps)
>> {
>>     size_t i;
>> -    size_t j;
>> -    char *siblings;
>>
>>     if (!caps)
>>     return 0;
>> @@ -819,6 +817,8 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
>>     virBufferAdjustIndent(buf, 2);
>>     for (i = 0; i < caps->cells->len; i++) {
>>     virCapsHostNUMACell *cell = g_ptr_array_index(caps->cells, i);
>> +    size_t j;
>> +
>>     virBufferAsprintf(buf, "\n", cell->num);
>>     virBufferAdjustIndent(buf, 2);
>>
>> @@ -847,10 +847,12 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
>>
>>     virBufferAsprintf(buf, "\n", cell->ncpus);
>>     virBufferAdjustIndent(buf, 2);
>> -    for (j = 0; j < cell->ncpus; j++) {
>> +    for (j = 0; j < cell->ncpus; ++j) {
> 
> No need to change the post-increment to pre-increment.

Huh, I have no idea how that slipped in there :-)

Michal



Re: [libvirt PATCH] meson: Add yajl kludge

2021-05-17 Thread Ján Tomko

On a Friday in 2021, Andrea Bolognani wrote:

If this looks familiar, that's because it's literally *the
same code* that we used to work around *the same issue* in
readline before 1635dca26f61def3fbf56c70fbbfe514f2b50987 :)

Note that the issue only really affects people building from
source on Apple Silicon: on Intel, Homebrew installs header
files under directories that are part of the default search
path, which explains why our CI pipeline never ran into it.

Signed-off-by: Andrea Bolognani 
---
Roman, can you please test this? Thanks! :)

meson.build | 35 +++
1 file changed, 35 insertions(+)



Reviewed-by: Ján Tomko 

(not tested, obviously)

Jano


signature.asc
Description: PGP signature


Re: [PATCH 0/4] Couple of virnuma cleanups

2021-05-17 Thread Ján Tomko

On a Monday in 2021, Michal Privoznik wrote:

I'm looking into NUMA code in capabilities and came up with a couple of
cleanups. Technically, 3/4 is not needed yet, but I'll be introducing
new data to vircaps2xmltest where a NUMA node has no CPUs and that's
why the patch is needed.

Michal Prívozník (4):
 numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML
 virnuma: Export virNumaGetMaxCPUs properly
 virnumamock: Allow CPU-less NUMA nodes
 virCapabilitiesHostNUMAFormat: Bring variables into loops

src/conf/capabilities.c  |   9 ++--
src/conf/numa_conf.c | 107 ++-
src/libvirt_private.syms |   1 +
src/util/virnuma.h   |   2 +-
tests/virnumamock.c  |   7 ++-
5 files changed, 61 insertions(+), 65 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH 4/4] virCapabilitiesHostNUMAFormat: Bring variables into loops

2021-05-17 Thread Ján Tomko

On a Monday in 2021, Michal Privoznik wrote:

Signed-off-by: Michal Privoznik 
---
src/conf/capabilities.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 1dae6d38cc..915cd3149e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -807,8 +807,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
  virCapsHostNUMA *caps)
{
size_t i;
-size_t j;
-char *siblings;

if (!caps)
return 0;
@@ -819,6 +817,8 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
virBufferAdjustIndent(buf, 2);
for (i = 0; i < caps->cells->len; i++) {
virCapsHostNUMACell *cell = g_ptr_array_index(caps->cells, i);
+size_t j;
+
virBufferAsprintf(buf, "\n", cell->num);
virBufferAdjustIndent(buf, 2);

@@ -847,10 +847,12 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,

virBufferAsprintf(buf, "\n", cell->ncpus);
virBufferAdjustIndent(buf, 2);
-for (j = 0; j < cell->ncpus; j++) {
+for (j = 0; j < cell->ncpus; ++j) {


No need to change the post-increment to pre-increment.

Jano


virBufferAsprintf(buf, "cpus[j].id);

if (cell->cpus[j].siblings) {
+g_autofree char * siblings = NULL;
+
if (!(siblings = virBitmapFormat(cell->cpus[j].siblings)))
return -1;



signature.asc
Description: PGP signature


Re: [PATCH v1 5/7] docs: mark intention to deprecate TCG tracing functionality

2021-05-17 Thread Stefan Hajnoczi
On Mon, May 17, 2021 at 11:47:11AM +0100, Alex Bennée wrote:
> 
> Daniel P. Berrangé  writes:
> 
> > On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:
> >> 
> >> Alex Bennée  writes:
> >> 
> >> > Daniel P. Berrangé  writes:
> >> >
> >> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
> >> 
> >> >>> +TCG introspection features
> >> >>> +--
> >> >>> +
> >> >>> +TCG trace-events (since 6.1)
> >> >>> +
> >> >>> +
> >> >>> +The ability to add new TCG trace points has bit rotted and as the
> >> >>
> >> >> When you say this "has bit rotted", just how bad is the situation ?
> >> >>
> >> >> Is the TCG tracing still usable at all, or is is fully broken
> >> >> already ?
> >> >
> >> > Well patches 6/7 got it working for generic TCG things. I haven't been
> >> > able to get the architecture one working but I suspect that is some sort
> >> > of interaction between the per-arch trace header generation that I
> >> > haven't quite figured out yet.
> >> 
> >> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
> >> limited tcg/vcpu events to the root trace-events file.
> >
> > That commit is from release 2.10.0.
> >
> > The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.
> >
> > So no one has been able to use this feature for 3+ years already.
> >
> > Is it actually worth fixing and then deprecating for 2 releases before
> > deleting, as opposed to just deleting the broken code today on basis
> > that it can't have any current users ?
> 
> Well I can get it up and running with the aforementioned patches and it
> seems reasonable to give some notice. I'm happy to defer to Stefan here
> though as it's his sub-system.

Lluís Vilanova was the author and probably main user. He mentioned he's
been away from QEMU for a while.

If you want to drop the feature, I think that's fine since it has
already been broken for over 3 years. If someone wants it back then it
can be added via TCG plugins in the future.

Stefan


signature.asc
Description: PGP signature


Re: [PATCH] driver: Don't leak saved error in virGetConnectGeneric()

2021-05-17 Thread Ján Tomko

On a Monday in 2021, Michal Privoznik wrote:

Recently, a new code was added to virGetConnectGeneric() that
saves the original error into a variable so that it's not lost in
virConnectClose() called under the 'error' label.

However, the error saving code uses virSaveLastError() +
virSetError() combo which leaks the memory allocated for the
error copy. Using virErrorPreserveLast() + virErrorRestore() does
the same job without the memleak.

Signed-off-by: Michal Privoznik 
---
src/driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


[PATCH] driver: Don't leak saved error in virGetConnectGeneric()

2021-05-17 Thread Michal Privoznik
Recently, a new code was added to virGetConnectGeneric() that
saves the original error into a variable so that it's not lost in
virConnectClose() called under the 'error' label.

However, the error saving code uses virSaveLastError() +
virSetError() combo which leaks the memory allocated for the
error copy. Using virErrorPreserveLast() + virErrorRestore() does
the same job without the memleak.

Signed-off-by: Michal Privoznik 
---
 src/driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/driver.c b/src/driver.c
index 227bb56e48..329d493a50 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -138,7 +138,7 @@ static virConnectPtr
 virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
 {
 virConnectPtr conn;
-virErrorPtr saved;
+virErrorPtr orig_err;
 
 if (virConnectCacheInitialize() < 0)
 return NULL;
@@ -178,9 +178,9 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char 
*name)
 return conn;
 
  error:
-saved = virSaveLastError();
+virErrorPreserveLast(_err);
 virConnectClose(conn);
-virSetError(saved);
+virErrorRestore(_err);
 return NULL;
 }
 
-- 
2.26.3



[PATCH 4/4] virCapabilitiesHostNUMAFormat: Bring variables into loops

2021-05-17 Thread Michal Privoznik
Signed-off-by: Michal Privoznik 
---
 src/conf/capabilities.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 1dae6d38cc..915cd3149e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -807,8 +807,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
   virCapsHostNUMA *caps)
 {
 size_t i;
-size_t j;
-char *siblings;
 
 if (!caps)
 return 0;
@@ -819,6 +817,8 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
 virBufferAdjustIndent(buf, 2);
 for (i = 0; i < caps->cells->len; i++) {
 virCapsHostNUMACell *cell = g_ptr_array_index(caps->cells, i);
+size_t j;
+
 virBufferAsprintf(buf, "\n", cell->num);
 virBufferAdjustIndent(buf, 2);
 
@@ -847,10 +847,12 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
 
 virBufferAsprintf(buf, "\n", cell->ncpus);
 virBufferAdjustIndent(buf, 2);
-for (j = 0; j < cell->ncpus; j++) {
+for (j = 0; j < cell->ncpus; ++j) {
 virBufferAsprintf(buf, "cpus[j].id);
 
 if (cell->cpus[j].siblings) {
+g_autofree char * siblings = NULL;
+
 if (!(siblings = virBitmapFormat(cell->cpus[j].siblings)))
 return -1;
 
@@ -860,7 +862,6 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf,
   cell->cpus[j].die_id,
   cell->cpus[j].core_id,
   siblings);
-VIR_FREE(siblings);
 }
 virBufferAddLit(buf, "/>\n");
 }
-- 
2.26.3



[PATCH 3/4] virnumamock: Allow CPU-less NUMA nodes

2021-05-17 Thread Michal Privoznik
The original virNumaGetNodeCPUs() returns an empty virBitmap if
given NUMA node has no CPUs. But that's not how our mock behaves
- it looks under $fakesysfs/node/node$N/cpulist only to find an
empty file which is then passed to virBitmapParseUnlimited()
which threats such input as error.

Fortunately, we don't have any fake sysfs data where this path is
hit, but we might soon.

Signed-off-by: Michal Privoznik 
---
 tests/virnumamock.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index 3a203ded77..ff9c6e951d 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -172,7 +172,12 @@ virNumaGetNodeCPUs(int node, virBitmap **cpus)
SYSFS_SYSTEM_PATH, node) < 0)
 return -1;
 
-*cpus = virBitmapParseUnlimited(cpulist);
+if (STREQ(cpulist, "")) {
+unsigned int max_n_cpus = virNumaGetMaxCPUs();
+*cpus = virBitmapNew(max_n_cpus);
+} else {
+*cpus = virBitmapParseUnlimited(cpulist);
+}
 if (!*cpus)
 goto cleanup;
 
-- 
2.26.3



[PATCH 0/4] Couple of virnuma cleanups

2021-05-17 Thread Michal Privoznik
I'm looking into NUMA code in capabilities and came up with a couple of
cleanups. Technically, 3/4 is not needed yet, but I'll be introducing
new data to vircaps2xmltest where a NUMA node has no CPUs and that's
why the patch is needed.

Michal Prívozník (4):
  numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML
  virnuma: Export virNumaGetMaxCPUs properly
  virnumamock: Allow CPU-less NUMA nodes
  virCapabilitiesHostNUMAFormat: Bring variables into loops

 src/conf/capabilities.c  |   9 ++--
 src/conf/numa_conf.c | 107 ++-
 src/libvirt_private.syms |   1 +
 src/util/virnuma.h   |   2 +-
 tests/virnumamock.c  |   7 ++-
 5 files changed, 61 insertions(+), 65 deletions(-)

-- 
2.26.3



[PATCH 1/4] numa_conf: Use virXMLFormatElement() in virDomainNumaDefFormatXML

2021-05-17 Thread Michal Privoznik
Signed-off-by: Michal Privoznik 
---
 src/conf/numa_conf.c | 107 +++
 1 file changed, 48 insertions(+), 59 deletions(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 525bc28962..537e515b5a 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -1105,92 +1105,81 @@ virDomainNumaDefFormatXML(virBuffer *buf,
 virBufferAdjustIndent(buf, 2);
 for (i = 0; i < ncells; i++) {
 virBitmap *cpumask = virDomainNumaGetNodeCpumask(def, i);
-int ndistances;
-size_t ncaches;
+g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
+g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+size_t j;
 
 memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i);
 discard = virDomainNumaGetNodeDiscard(def, i);
 
-virBufferAddLit(buf, "mem_nodes[i].ndistances;
-ncaches = def->mem_nodes[i].ncaches;
-if (ndistances == 0 && ncaches == 0) {
-virBufferAddLit(buf, "/>\n");
-} else {
-size_t j;
-
-virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
-
-if (ndistances) {
-virDomainNumaDistance *distances = def->mem_nodes[i].distances;
-
-virBufferAddLit(buf, "\n");
-virBufferAdjustIndent(buf, 2);
-for (j = 0; j < ndistances; j++) {
-if (distances[j].value) {
-virBufferAddLit(buf, "\n");
-}
+if (def->mem_nodes[i].ndistances) {
+virDomainNumaDistance *distances = def->mem_nodes[i].distances;
+
+virBufferAddLit(, "\n");
+virBufferAdjustIndent(, 2);
+for (j = 0; j < def->mem_nodes[i].ndistances; j++) {
+if (distances[j].value) {
+virBufferAddLit(, "\n");
 }
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
 }
+virBufferAdjustIndent(, -2);
+virBufferAddLit(, "\n");
+}
 
-for (j = 0; j < ncaches; j++) {
-virDomainNumaCache *cache = >mem_nodes[i].caches[j];
-
-virBufferAsprintf(buf, "level);
-if (cache->associativity) {
-virBufferAsprintf(buf, " associativity='%s'",
-  
virDomainCacheAssociativityTypeToString(cache->associativity));
-}
+for (j = 0; j < def->mem_nodes[i].ncaches; j++) {
+virDomainNumaCache *cache = >mem_nodes[i].caches[j];
 
-if (cache->policy) {
-virBufferAsprintf(buf, " policy='%s'",
-  
virDomainCachePolicyTypeToString(cache->policy));
-}
-virBufferAddLit(buf, ">\n");
+virBufferAsprintf(, "level);
+if (cache->associativity) {
+virBufferAsprintf(, " associativity='%s'",
+  
virDomainCacheAssociativityTypeToString(cache->associativity));
+}
 
-virBufferAdjustIndent(buf, 2);
-virBufferAsprintf(buf,
-  "\n",
-  cache->size);
+if (cache->policy) {
+virBufferAsprintf(, " policy='%s'",
+  
virDomainCachePolicyTypeToString(cache->policy));
+}
+virBufferAddLit(, ">\n");
 
-if (cache->line) {
-virBufferAsprintf(buf,
-  "\n",
-  cache->line);
-}
+virBufferAdjustIndent(, 2);
+virBufferAsprintf(,
+  "\n",
+  cache->size);
 
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+if (cache->line) {
+virBufferAsprintf(,
+  "\n",
+  cache->line);
 }
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+
+virBufferAdjustIndent(, -2);
+virBufferAddLit(, "\n");
 }
+
+virXMLFormatElement(buf, "cell", , );
 }
 
 if (def->ninterconnects) {
-- 
2.26.3



[PATCH 2/4] virnuma: Export virNumaGetMaxCPUs properly

2021-05-17 Thread Michal Privoznik
This function will be used in virnumamock, shortly.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms | 1 +
 src/util/virnuma.h   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1df4b8cfe8..b7bbe46dc7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2872,6 +2872,7 @@ virNodeSuspendGetTargetMask;
 virNumaGetAutoPlacementAdvice;
 virNumaGetDistances;
 virNumaGetHostMemoryNodeset;
+virNumaGetMaxCPUs;
 virNumaGetMaxNode;
 virNumaGetNodeCPUs;
 virNumaGetNodeMemory;
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 45342ecf11..bc79bff891 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -43,7 +43,7 @@ int virNumaGetNodeMemory(int node,
  unsigned long long *memsize,
  unsigned long long *memfree) G_GNUC_NO_INLINE;
 
-unsigned int virNumaGetMaxCPUs(void);
+unsigned int virNumaGetMaxCPUs(void) G_GNUC_NO_INLINE;
 
 int virNumaGetNodeCPUs(int node, virBitmap **cpus) G_GNUC_NO_INLINE;
 int virNumaNodesetToCPUset(virBitmap *nodeset,
-- 
2.26.3



Re: [PATCH v1 5/7] docs: mark intention to deprecate TCG tracing functionality

2021-05-17 Thread Alex Bennée


Daniel P. Berrangé  writes:

> On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:
>> 
>> Alex Bennée  writes:
>> 
>> > Daniel P. Berrangé  writes:
>> >
>> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
>> 
>> >>> +TCG introspection features
>> >>> +--
>> >>> +
>> >>> +TCG trace-events (since 6.1)
>> >>> +
>> >>> +
>> >>> +The ability to add new TCG trace points has bit rotted and as the
>> >>
>> >> When you say this "has bit rotted", just how bad is the situation ?
>> >>
>> >> Is the TCG tracing still usable at all, or is is fully broken
>> >> already ?
>> >
>> > Well patches 6/7 got it working for generic TCG things. I haven't been
>> > able to get the architecture one working but I suspect that is some sort
>> > of interaction between the per-arch trace header generation that I
>> > haven't quite figured out yet.
>> 
>> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
>> limited tcg/vcpu events to the root trace-events file.
>
> That commit is from release 2.10.0.
>
> The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.
>
> So no one has been able to use this feature for 3+ years already.
>
> Is it actually worth fixing and then deprecating for 2 releases before
> deleting, as opposed to just deleting the broken code today on basis
> that it can't have any current users ?

Well I can get it up and running with the aforementioned patches and it
seems reasonable to give some notice. I'm happy to defer to Stefan here
though as it's his sub-system.

>
> Regards,
> Daniel


-- 
Alex Bennée




Re: [PATCH 4/4] tests: qemucapabilities: Add test-data for the qemu-6.1 cycle

2021-05-17 Thread Jiri Denemark
On Mon, May 17, 2021 at 11:16:35 +0200, Peter Krempa wrote:
> Add test data based on qemu commit v6.0.0-540-g6005ee07c3.
> 
> Notable changes are the removal of 'sheepdog' disk storage protocol.
> 
> Additionally the cpu model reported when probing seems to have changed
> from:
> 
> "model-id": "AMD Ryzen 9 3900X 12-Core Processor"
> 
> to:
> 
> "model-id": "QEMU TCG CPU version 2.5+"

This is indeed strange as KVM used to report the host's CPU name
directly, but it shouldn't cause any harm except for confusion humans
looking at the libvirtd logs or capabilities cache. In other words, it's
worth fixing unless it was an intentional change with a very reason
behind.

> despite building on the same machine. This probably also results in the
> 2 test changes in the CPU definition which popped up in this update.

I'm not sure if it's related (i.e., caused by the same QEMU changes),
but it's definitely not caused by the difference in model-id. Libvirt
does not even look at model-id, we only check individual CPU properties.

QEMU just reports svm, npt, and nrip-save are disabled... might be worth
reporting to them to check whether it is expected.

Jirka



Re: [libvirt PATCH 1/4] test: move nodedev xml2xml output to a separate dir

2021-05-17 Thread Pavel Hrdina
On Fri, May 14, 2021 at 04:28:58PM -0500, Jonathon Jongsma wrote:
> Currently, we're loading and parsing the xml from the input file, and
> then formatting it and then comparing it directly back to the input
> file. This works for now, but is severely limiting as it relies on the
> input file being fully-specified and in the exact order as the output
> xml format.
> 
> If optional elements are ommitted in the input XML, the output xml
> may include default values for the ommitted elements and thus the output
> will not match the input.
> 
> In order to allow more flexibility in testing, save the expected output
> to a seprate 'out' directory similar to what most of the other xml2xml
> tests are already doing.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  tests/nodedevxml2xmlout/DVD_GCC_4247N.xml | 15 +
>  tests/nodedevxml2xmlout/DVD_with_media.xml| 18 ++
>  tests/nodedevxml2xmlout/ap_07_0038.xml|  9 +
>  tests/nodedevxml2xmlout/ap_card07.xml |  8 +
>  tests/nodedevxml2xmlout/ap_matrix.xml |  7 
>  .../ap_matrix_mdev_types.xml  | 14 
>  tests/nodedevxml2xmlout/ccw_0_0_.xml  | 10 ++
>  tests/nodedevxml2xmlout/computer.xml  | 16 +
>  .../css_0_0_fffe_mdev_types.xml   | 17 ++
>  tests/nodedevxml2xmlout/css_0_0_.xml  | 10 ++
>  tests/nodedevxml2xmlout/drm_renderD129.xml| 10 ++
>  ...v_3627463d_b7f0_4fea_b468_f1da537d301b.xml |  8 +
>  ...v_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml |  9 +
>  .../net_00_13_02_b9_f9_d3.xml | 21 
>  .../net_00_15_58_2f_e9_55.xml | 21 
>  .../pci__00_02_0_header_type.xml  | 16 +
>  .../pci__00_1c_0_header_type.xml  | 21 
>  .../pci__02_10_7_mdev_types.xml   | 33 +++
>  .../pci__02_10_7_sriov.xml| 24 ++
>  .../pci__02_10_7_sriov_pf_vfs_all.xml | 29 
>  ...0_02_10_7_sriov_pf_vfs_all_header_type.xml | 31 +
>  .../pci__02_10_7_sriov_vfs.xml| 27 +++
>  ...__02_10_7_sriov_zero_vfs_max_count.xml | 22 +
>  tests/nodedevxml2xmlout/pci_1002_71c4.xml | 14 
>  .../pci_8086_0c0c_snd_hda_intel.xml   | 17 ++
>  .../pci_8086_10c9_sriov_pf.xml| 18 ++
>  .../pci_8086_27c5_scsi_host.xml   |  7 
>  .../pci_8086_27c5_scsi_host_0.xml |  7 
>  .../pci_8086_27c5_scsi_host_0_unique_id.xml   |  8 +
>  ...i_8086_27c5_scsi_host_scsi_device_lun0.xml | 11 +++
>  .../pci_8086_27c5_scsi_host_scsi_host.xml |  7 
>  .../pci_8086_4238_pcie_wireless.xml   | 27 +++
>  tests/nodedevxml2xmlout/scsi_target0_0_0.xml  |  7 
>  tests/nodedevxml2xmlout/scsi_target1_0_0.xml  | 12 +++
>  ...rial_3600c0ff000d7a2a5d463ff490200.xml | 19 +++
>  ...al_SATA_HTS721010G9SA00_MPCZ12Y0GNGWSE.xml | 14 
>  .../usb_device_1d6b_1__00_1d_0.xml| 10 ++
>  .../usb_device_1d6b_1__00_1d_0_if0.xml| 10 ++
>  tests/nodedevxml2xmltest.c| 12 ---
>  39 files changed, 591 insertions(+), 5 deletions(-)
>  create mode 100644 tests/nodedevxml2xmlout/DVD_GCC_4247N.xml
>  create mode 100644 tests/nodedevxml2xmlout/DVD_with_media.xml
>  create mode 100644 tests/nodedevxml2xmlout/ap_07_0038.xml
>  create mode 100644 tests/nodedevxml2xmlout/ap_card07.xml
>  create mode 100644 tests/nodedevxml2xmlout/ap_matrix.xml
>  create mode 100644 tests/nodedevxml2xmlout/ap_matrix_mdev_types.xml
>  create mode 100644 tests/nodedevxml2xmlout/ccw_0_0_.xml
>  create mode 100644 tests/nodedevxml2xmlout/computer.xml
>  create mode 100644 tests/nodedevxml2xmlout/css_0_0_fffe_mdev_types.xml
>  create mode 100644 tests/nodedevxml2xmlout/css_0_0_.xml
>  create mode 100644 tests/nodedevxml2xmlout/drm_renderD129.xml
>  create mode 100644 
> tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml
>  create mode 100644 
> tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad.xml
>  create mode 100644 tests/nodedevxml2xmlout/net_00_13_02_b9_f9_d3.xml
>  create mode 100644 tests/nodedevxml2xmlout/net_00_15_58_2f_e9_55.xml
>  create mode 100644 tests/nodedevxml2xmlout/pci__00_02_0_header_type.xml
>  create mode 100644 tests/nodedevxml2xmlout/pci__00_1c_0_header_type.xml
>  create mode 100644 tests/nodedevxml2xmlout/pci__02_10_7_mdev_types.xml
>  create mode 100644 tests/nodedevxml2xmlout/pci__02_10_7_sriov.xml
>  create mode 100644 
> tests/nodedevxml2xmlout/pci__02_10_7_sriov_pf_vfs_all.xml
>  create mode 100644 
> tests/nodedevxml2xmlout/pci__02_10_7_sriov_pf_vfs_all_header_type.xml
>  create mode 100644 tests/nodedevxml2xmlout/pci__02_10_7_sriov_vfs.xml
>  create mode 100644 
> 

Re: [PATCH 0/4] tests: qemucapabilities: Add data fro the 6.1 cycle

2021-05-17 Thread Pavel Hrdina
On Mon, May 17, 2021 at 11:16:31AM +0200, Peter Krempa wrote:
> QEMU already committed some significant changes since the tree opened.
> The most notable which has fallout in libvirt is the dropping of the
> 'sheepdog' driver. This series adapts to that and then adds the
> qemu capabilities data for this cycle based on the most recent qemu
> upstream commit.
> 
> Patch 4 is heavily truncated. To fetch the full version please:
> 
> git fetch https://gitlab.com/pipo.sk/libvirt.git qemu-caps-6.1
> 
> Peter Krempa (4):
>   testQemuInfoSetArgs: Strip default machine alias only for 'latest'
> test cases
>   qemublocktest: Drop 'network-sheepdog-qcow2' image creation test case
>   qemuxml2argvtest: Limit 'disk-network-sheepdog' testcase to qemu-6.0.0
>   tests: qemucapabilities: Add test-data for the qemu-6.1 cycle

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature


Re: [PATCH] Delete forward declaration in storage_backend_sheepdog.c.

2021-05-17 Thread Peter Krempa
On Sun, May 16, 2021 at 14:23:07 +0800, peili wrote:
> ---
>  src/storage/storage_backend_sheepdog.c | 99 +-
>  1 file changed, 48 insertions(+), 51 deletions(-)

Note that we are considering fully removing the sheepdog storage driver
backend as of:

https://listman.redhat.com/archives/libvir-list/2021-April/msg00773.html

Pavel will be sending a v2 of that soon, so this patch would become
pointless.

If you are using the backend, please respond to the original thread with
a justification on why we shouldn't remove it, but the justification
would mostly require that sheepdog will be again maintained upstream.



[PATCH 4/4] tests: qemucapabilities: Add test-data for the qemu-6.1 cycle

2021-05-17 Thread Peter Krempa
Add test data based on qemu commit v6.0.0-540-g6005ee07c3.

Notable changes are the removal of 'sheepdog' disk storage protocol.

Additionally the cpu model reported when probing seems to have changed
from:

"model-id": "AMD Ryzen 9 3900X 12-Core Processor"

to:

"model-id": "QEMU TCG CPU version 2.5+"

despite building on the same machine. This probably also results in the
2 test changes in the CPU definition which popped up in this update.

Signed-off-by: Peter Krempa 
---

Note that this commit is snipped. See cover letter on where to fetch the
full version.

 .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml  |   208 +
 .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml  |   211 +
 tests/domaincapsdata/qemu_6.1.0.x86_64.xml|   208 +
 .../caps_6.1.0.x86_64.replies | 32734 
 .../caps_6.1.0.x86_64.xml |  3339 ++
 .../cpu-tsc-high-frequency.x86_64-latest.args | 2 +-
 .../hugepages-memaccess3.x86_64-latest.args   | 2 +-
 7 files changed, 36702 insertions(+), 2 deletions(-)
 create mode 100644 tests/domaincapsdata/qemu_6.1.0-q35.x86_64.xml
 create mode 100644 tests/domaincapsdata/qemu_6.1.0-tcg.x86_64.xml
 create mode 100644 tests/domaincapsdata/qemu_6.1.0.x86_64.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml

diff --git a/tests/domaincapsdata/qemu_6.1.0-q35.x86_64.xml 
b/tests/domaincapsdata/qemu_6.1.0-q35.x86_64.xml
new file mode 100644
index 00..95291c8430
--- /dev/null
+++ b/tests/domaincapsdata/qemu_6.1.0-q35.x86_64.xml
@@ -0,0 +1,208 @@
+
+  /usr/bin/qemu-system-x86_64
+  kvm
+  pc-q35-6.1
+  x86_64
+  
+  
+  
+
+  bios
+  efi
+
+
+  /usr/share/AAVMF/AAVMF_CODE.fd
+  /usr/share/AAVMF/AAVMF32_CODE.fd
+  /usr/share/OVMF/OVMF_CODE.fd
+  
+rom
+pflash
+  
+  
+yes
+no
+  
+  
+yes
+no
+  
+
+  
+  
+
+  
+on
+off
+  
+
+
+  
+on
+off
+  
+
+
+  EPYC-Rome
+  AMD
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+  qemu64
+  qemu32
+  phenom
+  pentium3
+  pentium2
+  pentium
+  n270
+  kvm64
+  kvm32
+  coreduo
+  core2duo
+  athlon
+  Westmere-IBRS
+  Westmere
+  Snowridge
+  Skylake-Server-noTSX-IBRS
+  Skylake-Server-IBRS
+  Skylake-Server
+  Skylake-Client-noTSX-IBRS
+  Skylake-Client-IBRS
+  Skylake-Client
+  SandyBridge-IBRS
+  SandyBridge
+  Penryn
+  Opteron_G5
+  Opteron_G4
+  Opteron_G3
+  Opteron_G2
+  Opteron_G1
+  Nehalem-IBRS
+  Nehalem
+  IvyBridge-IBRS
+  IvyBridge
+  Icelake-Server-noTSX
+  Icelake-Server
+  Icelake-Client-noTSX
+  Icelake-Client
+  Haswell-noTSX-IBRS
+  Haswell-noTSX
+  Haswell-IBRS
+  Haswell
+  EPYC-Rome
+  EPYC-Milan
+  EPYC-IBPB
+  EPYC
+  Dhyana
+  Cooperlake
+  Conroe
+  Cascadelake-Server-noTSX
+  Cascadelake-Server
+  Broadwell-noTSX-IBRS
+  Broadwell-noTSX
+  Broadwell-IBRS
+  Broadwell
+  486
+
+  
+  
+
+  
+disk
+cdrom
+floppy
+lun
+  
+  
+fdc
+scsi
+virtio
+usb
+sata
+  
+  
+virtio
+virtio-transitional
+virtio-non-transitional
+  
+
+
+  
+sdl
+vnc
+spice
+egl-headless
+  
+
+
+  
+vga
+cirrus
+vmvga
+qxl
+virtio
+none
+bochs
+ramfb
+  
+
+
+  
+subsystem
+  
+  
+default
+mandatory
+requisite
+optional
+  
+  
+usb
+pci
+scsi
+  
+  
+  
+default
+vfio
+  
+
+
+  
+virtio
+virtio-transitional
+virtio-non-transitional
+  
+  
+random
+egd
+builtin
+  
+
+  
+  
+
+
+
+
+
+
+  
+
diff --git a/tests/domaincapsdata/qemu_6.1.0-tcg.x86_64.xml 
b/tests/domaincapsdata/qemu_6.1.0-tcg.x86_64.xml
new file mode 100644
index 00..4e19eea365
--- /dev/null
+++ b/tests/domaincapsdata/qemu_6.1.0-tcg.x86_64.xml
@@ -0,0 +1,211 @@
+
+  /usr/bin/qemu-system-x86_64
+  qemu
+  pc-i440fx-6.1
+  x86_64
+  
+  
+  
+
+  bios
+  efi
+
+
+  /usr/share/AAVMF/AAVMF_CODE.fd
+  /usr/share/AAVMF/AAVMF32_CODE.fd
+  /usr/share/OVMF/OVMF_CODE.fd
+  
+rom
+pflash
+  
+  
+yes
+no
+  
+  
+no
+  
+
+  
+  
+
+
+  
+on
+  

[PATCH 1/4] testQemuInfoSetArgs: Strip default machine alias only for 'latest' test cases

2021-05-17 Thread Peter Krempa
For the real-capabilities test cases testing 'latest' capabilities we
strip off the alias from 'pc' to the appropriate versioned machine type
to prevent update to all tests when bumping qemu capabilities.

Recenly we also started caching the capabilities to prevent re-parsing
the XML all the time. The commit adding the caching kept the alias
stripping prior to cache insertion, thus the cache contains the stripped
alias.

This leads to problem when a test case is added where the 'latest'
equals to the selected version.

Move the machine alias stripping after we create a local copy thus
stripping it only for 'latest' tests.

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

diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 1a3eae2c07..1444abc401 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -790,9 +790,6 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
 if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, 
capsfile)))
 goto cleanup;

-if (stripmachinealiases)
-virQEMUCapsStripMachineAliases(qemuCaps);
-
 cachedcaps = qemuCaps;

 g_hash_table_insert(capscache, g_strdup(capsfile), 
g_steal_pointer());
@@ -801,6 +798,9 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
 if (!(qemuCaps = virQEMUCapsNewCopy(cachedcaps)))
 goto cleanup;

+if (stripmachinealiases)
+virQEMUCapsStripMachineAliases(qemuCaps);
+
 info->flags |= FLAG_REAL_CAPS;

 /* provide path to the replies file for schema testing */
-- 
2.30.2



[PATCH 3/4] qemuxml2argvtest: Limit 'disk-network-sheepdog' testcase to qemu-6.0.0

2021-05-17 Thread Peter Krempa
QEMU is dropping sheepdog support in 6.1 so we need to limit the test
case to the latest version supporting sheepdog as it won't be described
by the QMP schema any more.

Signed-off-by: Peter Krempa 
---
 ...6_64-latest.args => disk-network-sheepdog.x86_64-6.0.0.args} | 2 +-
 tests/qemuxml2argvtest.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename tests/qemuxml2argvdata/{disk-network-sheepdog.x86_64-latest.args => 
disk-network-sheepdog.x86_64-6.0.0.args} (95%)

diff --git a/tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-6.0.0.args
similarity index 95%
rename from tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-latest.args
rename to tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-6.0.0.args
index a0c663229c..078ad6be09 100644
--- a/tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-6.0.0.args
@@ -10,7 +10,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
 -name guest=QEMUGuest1,debug-threads=on \
 -S \
 -object 
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}'
 \
--machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-machine 
pc-i440fx-6.0,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \
 -cpu qemu64 \
 -m 214 \
 -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d5e59fe474..25b0c81f21 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1395,7 +1395,7 @@ mymain(void)
 DO_TEST_FAILURE("disk-network-rbd-no-colon", NONE);
 DO_TEST("disk-network-sheepdog", NONE);
 DO_TEST_CAPS_VER("disk-network-sheepdog", "2.12.0");
-DO_TEST_CAPS_LATEST("disk-network-sheepdog");
+DO_TEST_CAPS_VER("disk-network-sheepdog", "6.0.0");
 DO_TEST("disk-network-source-auth", NONE);
 DO_TEST_CAPS_VER("disk-network-source-auth", "2.12.0");
 DO_TEST_CAPS_LATEST("disk-network-source-auth");
-- 
2.30.2



[PATCH 2/4] qemublocktest: Drop 'network-sheepdog-qcow2' image creation test case

2021-05-17 Thread Peter Krempa
QEMU dropped sheepdog support for the 6.1 release. Since we use schema
validation in the image creation it would create test failures.

In this instance we just drop the test altogether as adding versioned
capabilities would be a bit too overkill for this scenario.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  1 -
 .../imagecreate/network-sheepdog-qcow2.json   | 20 ---
 .../imagecreate/network-sheepdog-qcow2.xml| 12 ---
 3 files changed, 33 deletions(-)
 delete mode 100644 
tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
 delete mode 100644 
tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.xml

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3f3e6d1532..0513e5bfc0 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1223,7 +1223,6 @@ mymain(void)
 TEST_IMAGE_CREATE("network-gluster-qcow2", NULL);
 TEST_IMAGE_CREATE("network-rbd-qcow2", NULL);
 TEST_IMAGE_CREATE("network-ssh-qcow2", NULL);
-TEST_IMAGE_CREATE("network-sheepdog-qcow2", NULL);

 #define TEST_BITMAP_DETECT(testname) \
 do { \
diff --git a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json 
b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
deleted file mode 100644
index d86bef6bc8..00
--- a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
+++ /dev/null
@@ -1,20 +0,0 @@
-protocol:
-{
-  "driver": "sheepdog",
-  "location": {
-"server": {
-  "type": "inet",
-  "host": "example.com",
-  "port": "1234"
-},
-"vdi": "asdf/i.qcow2"
-  },
-  "size": 4294967296
-}
-
-format:
-{
-  "driver": "qcow2",
-  "file": "0123456789ABCDEF0123456789ABCDE",
-  "size": 8589934590
-}
diff --git a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.xml 
b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.xml
deleted file mode 100644
index 1145daafdd..00
--- a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.xml
+++ /dev/null
@@ -1,12 +0,0 @@
-
-  
-  
-
-
-  
-
-
-  
-
-  
-
-- 
2.30.2



[PATCH 0/4] tests: qemucapabilities: Add data fro the 6.1 cycle

2021-05-17 Thread Peter Krempa
QEMU already committed some significant changes since the tree opened.
The most notable which has fallout in libvirt is the dropping of the
'sheepdog' driver. This series adapts to that and then adds the
qemu capabilities data for this cycle based on the most recent qemu
upstream commit.

Patch 4 is heavily truncated. To fetch the full version please:

git fetch https://gitlab.com/pipo.sk/libvirt.git qemu-caps-6.1

Peter Krempa (4):
  testQemuInfoSetArgs: Strip default machine alias only for 'latest'
test cases
  qemublocktest: Drop 'network-sheepdog-qcow2' image creation test case
  qemuxml2argvtest: Limit 'disk-network-sheepdog' testcase to qemu-6.0.0
  tests: qemucapabilities: Add test-data for the qemu-6.1 cycle

 .../domaincapsdata/qemu_6.1.0-q35.x86_64.xml  |   208 +
 .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml  |   211 +
 tests/domaincapsdata/qemu_6.1.0.x86_64.xml|   208 +
 tests/qemublocktest.c | 1 -
 .../imagecreate/network-sheepdog-qcow2.json   |20 -
 .../imagecreate/network-sheepdog-qcow2.xml|12 -
 .../caps_6.1.0.x86_64.replies | 32734 
 .../caps_6.1.0.x86_64.xml |  3339 ++
 .../cpu-tsc-high-frequency.x86_64-latest.args | 2 +-
 ...> disk-network-sheepdog.x86_64-6.0.0.args} | 2 +-
 .../hugepages-memaccess3.x86_64-latest.args   | 2 +-
 tests/qemuxml2argvtest.c  | 2 +-
 tests/testutilsqemu.c | 6 +-
 13 files changed, 36707 insertions(+), 40 deletions(-)
 create mode 100644 tests/domaincapsdata/qemu_6.1.0-q35.x86_64.xml
 create mode 100644 tests/domaincapsdata/qemu_6.1.0-tcg.x86_64.xml
 create mode 100644 tests/domaincapsdata/qemu_6.1.0.x86_64.xml
 delete mode 100644 
tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
 delete mode 100644 
tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
 rename tests/qemuxml2argvdata/{disk-network-sheepdog.x86_64-latest.args => 
disk-network-sheepdog.x86_64-6.0.0.args} (95%)

-- 
2.30.2



Re: [PATCH 0/3] tests: Return EXIT_* from main instead of 0/-1

2021-05-17 Thread Peter Krempa
On Mon, May 17, 2021 at 08:49:07 +0200, Michal Privoznik wrote:
> See 3/3 for rationale.
> 
> Michal Prívozník (3):
>   testutils: Drop libtool binary name handling
>   tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0
>   testutils: Document and enforce @func callback retvals for
> virTestMain()

Series:

Reviewed-by: Peter Krempa 

I've actually fixed a few instances before where we were returning -1
which was actually confusing the test suite and printing that the test
got some weird signal.



Re: [PATCH 3/3] testutils: Document and enforce @func callback retvals for virTestMain()

2021-05-17 Thread Peter Krempa
On Mon, May 17, 2021 at 08:49:10 +0200, Michal Privoznik wrote:
> Sometimes a test has a wrapper over main() (e.g. because it's
> preloading some mock libraries). In such case, the main() is
> renamed to something else (usually mymain()), and main() is
> generated by calling one of VIR_TEST_MAIN() or
> VIR_TEST_MAIN_PRELOAD() macros.

AFAIK it's not just sometimes but always in our testsuite.

> 
> This has a neat side effect - if mymain() returns an error a
> short summary is printed, e.g.:
> 
>   Some tests failed. Run them using:
>   VIR_TEST_DEBUG=1 VIR_TEST_RANGE=5-6 ./virtest
> 
> However, this detection only works if EXIT_FAILURE is returned by
> mymain(). Document and enforce this limitation.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  tests/testutils.c | 13 +
>  tests/testutils.h |  4 
>  2 files changed, 17 insertions(+)
> 
> diff --git a/tests/testutils.c b/tests/testutils.c
> index 870a3b081a..b57b44fab5 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -838,6 +838,19 @@ int virTestMain(int argc,
>  fprintf(stderr, "%*s", 40 - (int)(testCounter % 40), "");
>  fprintf(stderr, " %-3zu %s\n", testCounter, ret == 0 ? "OK" : 
> "FAIL");
>  }
> +
> +switch (ret) {
> +case EXIT_FAILURE:
> +case EXIT_SUCCESS:
> +case EXIT_AM_SKIP:
> +case EXIT_AM_HARDFAIL:
> +break;
> +default:
> +fprintf(stderr, "Test callback did returned invalid value: %d\n", 
> ret);

s/did//

> +ret = EXIT_AM_HARDFAIL;
> +break;
> +}



Re: How to hot plugin a new vhost-user-blk-pci device to running VM?

2021-05-17 Thread Michal Prívozník
On 5/17/21 7:08 AM, Liang Chaojun wrote:
> 
> Thanks Michal and Peter for your response. I‘ m running it on qemu 5.1 build 
> by myself. BTW, follow Peter’s suggestion, where I can get the latest rpms if 
> I want to upgrade to Libvirt 7.1? As I know it seems need more than twenty 
> related rpms not include dependency.

Well, since you're building qemu yourself you could also build libvirt.
Just be aware that the v7.3.0 release was the last one that supports
RHEL-7. Newer releases might still work, but upstream does not aim to
make everything work.

https://libvirt.org/platforms.html

Michal



Re: How to hot plugin a new vhost-user-blk-pci device to running VM?

2021-05-17 Thread Han Han
On Mon, May 17, 2021 at 1:09 PM Liang Chaojun 
wrote:

>
> Thanks Michal and Peter for your response. I‘ m running it on qemu 5.1
> build by myself. BTW, follow Peter’s suggestion, where I can get the latest
> rpms if I want to upgrade to Libvirt 7.1? As I know it seems need more than
> twenty related rpms not include dependency.
>
You can get the rpms from centos koji https://cbs.centos.org/koji/
https://koji.mbox.centos.org/koji/ or fedora koji
https://koji.fedoraproject.org/koji/
Since the version of rpm pkgs in RHEL7.4 are too low, I am afraid it will
be hard to install libvirt 7.1.
It better to install it in RHEL8 or Centos8

>
> > 在 2021年5月14日,21:15,Michal Prívozník  写道:
> >
> > On 5/14/21 8:33 AM, Liang Chaojun wrote:
> >>
> >>
> >> Thanks,I have tried qemu monitor as below. I used chardev_add to add a
> chardev and used device_add it to running vm. But i often hit a issue and
> cause the vm crash. qemu-system-x86_64: ../hw/virtio/vhost.c:1566:
> vhost_dev_get_config: Assertion `hdev->vhost_ops' failed.
> >>
> >> virsh qemu-monitor-command spdk1 --hmp --cmd "chardev-add
> socket,id=spdk_vhost_blk0,path=/var/tmp/vhost.0,reconnect=1"  virsh
> qemu-monitor-command spdk1 --hmp --cmd "device_add
> vhost-user-blk-pci,chardev=spdk_vhost_blk0,num-queues=4"
> >
> > Smells like problem described here:
> >
> >
> https://gitlab.com/qemu-project/qemu/-/commit/bc79c87bcde6587a37347f81332fbb0cd6b14b85
> >
> > But that's pretty fresh commit (part of qemu 6.0.0 release). And also
> > the commit it references is not that old (qemu 5.1.0 release). What
> > version of qemu are
> >
>
>
>


[PATCH 3/3] testutils: Document and enforce @func callback retvals for virTestMain()

2021-05-17 Thread Michal Privoznik
Sometimes a test has a wrapper over main() (e.g. because it's
preloading some mock libraries). In such case, the main() is
renamed to something else (usually mymain()), and main() is
generated by calling one of VIR_TEST_MAIN() or
VIR_TEST_MAIN_PRELOAD() macros.

This has a neat side effect - if mymain() returns an error a
short summary is printed, e.g.:

  Some tests failed. Run them using:
  VIR_TEST_DEBUG=1 VIR_TEST_RANGE=5-6 ./virtest

However, this detection only works if EXIT_FAILURE is returned by
mymain(). Document and enforce this limitation.

Signed-off-by: Michal Privoznik 
---
 tests/testutils.c | 13 +
 tests/testutils.h |  4 
 2 files changed, 17 insertions(+)

diff --git a/tests/testutils.c b/tests/testutils.c
index 870a3b081a..b57b44fab5 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -838,6 +838,19 @@ int virTestMain(int argc,
 fprintf(stderr, "%*s", 40 - (int)(testCounter % 40), "");
 fprintf(stderr, " %-3zu %s\n", testCounter, ret == 0 ? "OK" : "FAIL");
 }
+
+switch (ret) {
+case EXIT_FAILURE:
+case EXIT_SUCCESS:
+case EXIT_AM_SKIP:
+case EXIT_AM_HARDFAIL:
+break;
+default:
+fprintf(stderr, "Test callback did returned invalid value: %d\n", ret);
+ret = EXIT_AM_HARDFAIL;
+break;
+}
+
 if (ret == EXIT_FAILURE && !virBitmapIsAllClear(failedTests)) {
 g_autofree char *failed = virBitmapFormat(failedTests);
 fprintf(stderr, "Some tests failed. Run them using:\n");
diff --git a/tests/testutils.h b/tests/testutils.h
index e268a95612..6848323586 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -98,6 +98,10 @@ void virTestQuiesceLibvirtErrors(bool always);
 void virTestCounterReset(const char *prefix);
 const char *virTestCounterNext(void);
 
+/**
+ * The @func shall return  EXIT_FAILURE or EXIT_SUCCESS or
+ * EXIT_AM_SKIP or EXIT_AM_HARDFAIL.
+ */
 int virTestMain(int argc,
 char **argv,
 int (*func)(void),
-- 
2.26.3



[PATCH 1/3] testutils: Drop libtool binary name handling

2021-05-17 Thread Michal Privoznik
Back in the old days, we used to use libtool to run compiled
libraries. That meant we had to deal with "lt-" prefix for our
binaries. With meson that's no longer the case.

Signed-off-by: Michal Privoznik 
---
 tests/testutils.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index d6b7c2a580..870a3b081a 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -750,8 +750,7 @@ int virTestMain(int argc,
 size_t noutputs = 0;
 virLogOutput *output = NULL;
 virLogOutput **outputs = NULL;
-g_autofree char *baseprogname = NULL;
-const char *progname;
+g_autofree char *progname = NULL;
 g_autofree const char **preloads = NULL;
 size_t npreloads = 0;
 g_autofree char *mock = NULL;
@@ -784,9 +783,7 @@ int virTestMain(int argc,
 VIR_TEST_PRELOAD(mock);
 }
 
-progname = baseprogname = g_path_get_basename(argv[0]);
-if (STRPREFIX(progname, "lt-"))
-progname += 3;
+progname = g_path_get_basename(argv[0]);
 
 g_setenv("VIR_TEST_MOCK_PROGNAME", progname, TRUE);
 
-- 
2.26.3



[PATCH 0/3] tests: Return EXIT_* from main instead of 0/-1

2021-05-17 Thread Michal Privoznik
See 3/3 for rationale.

Michal Prívozník (3):
  testutils: Drop libtool binary name handling
  tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0
  testutils: Document and enforce @func callback retvals for
virTestMain()

 tests/fchosttest.c  |  2 +-
 tests/qemusecuritytest.c|  2 +-
 tests/scsihosttest.c|  2 +-
 tests/seclabeltest.c|  2 +-
 tests/storagepoolcapstest.c |  2 +-
 tests/testutils.c   | 20 +++-
 tests/testutils.h   |  4 
 tests/virbitmaptest.c   |  2 +-
 tests/vircaps2xmltest.c |  2 +-
 tests/vircapstest.c |  2 +-
 tests/virconftest.c |  2 +-
 tests/virendiantest.c   |  2 +-
 tests/virlogtest.c  |  2 +-
 tests/virresctrltest.c  |  2 +-
 tests/virscsitest.c |  2 +-
 15 files changed, 32 insertions(+), 18 deletions(-)

-- 
2.26.3



[PATCH 2/3] tests: Return EXIT_FAILURE/EXIT_SUCCESS instead of -1/0

2021-05-17 Thread Michal Privoznik
When using VIR_TEST_MAIN() or VIR_TEST_MAIN_PRELOAD() macros, the
retval of mymain() will become retval of main(). Hence, mymain()
should use EXIT_FAILURE and EXIT_SUCCESS return values for
greater portability. Another reason is that otherwise our summary
printing of failed tests doesn't work (see following commit for
more info).

Signed-off-by: Michal Privoznik 
---
 tests/fchosttest.c  | 2 +-
 tests/qemusecuritytest.c| 2 +-
 tests/scsihosttest.c| 2 +-
 tests/seclabeltest.c| 2 +-
 tests/storagepoolcapstest.c | 2 +-
 tests/virbitmaptest.c   | 2 +-
 tests/vircaps2xmltest.c | 2 +-
 tests/vircapstest.c | 2 +-
 tests/virconftest.c | 2 +-
 tests/virendiantest.c   | 2 +-
 tests/virlogtest.c  | 2 +-
 tests/virresctrltest.c  | 2 +-
 tests/virscsitest.c | 2 +-
 13 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tests/fchosttest.c b/tests/fchosttest.c
index 44e7f11599..53d02241ca 100644
--- a/tests/fchosttest.c
+++ b/tests/fchosttest.c
@@ -374,7 +374,7 @@ mymain(void)
 ret = -1;
 
 VIR_FREE(fchost_prefix);
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virrandom"))
diff --git a/tests/qemusecuritytest.c b/tests/qemusecuritytest.c
index 184ffca15f..f7186700c4 100644
--- a/tests/qemusecuritytest.c
+++ b/tests/qemusecuritytest.c
@@ -261,7 +261,7 @@ mymain(void)
 #endif
 virObjectUnref(dac);
 virObjectUnref(stack);
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN_PRELOAD(mymain,
diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c
index 17825bba35..7508ac37a3 100644
--- a/tests/scsihosttest.c
+++ b/tests/scsihosttest.c
@@ -282,7 +282,7 @@ mymain(void)
 VIR_FREE(fakerootdir);
 VIR_FREE(fakesysfsdir);
 VIR_FREE(scsihost_class_path);
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c
index 39a96d0fc0..b2a11b278c 100644
--- a/tests/seclabeltest.c
+++ b/tests/seclabeltest.c
@@ -33,7 +33,7 @@ mymain(void)
 
 virObjectUnref(mgr);
 
-return 0;
+return EXIT_SUCCESS;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/storagepoolcapstest.c b/tests/storagepoolcapstest.c
index f937670aa7..526c9ad045 100644
--- a/tests/storagepoolcapstest.c
+++ b/tests/storagepoolcapstest.c
@@ -101,7 +101,7 @@ mymain(void)
 DO_TEST("full", fullCaps);
 DO_TEST("fs", fsCaps);
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index a376a613db..02241c4c20 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -784,7 +784,7 @@ mymain(void)
 if (virTestRun("test16", test16, NULL) < 0)
 ret = -1;
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c
index ead3fb88ff..5b1b60124a 100644
--- a/tests/vircaps2xmltest.c
+++ b/tests/vircaps2xmltest.c
@@ -113,7 +113,7 @@ mymain(void)
 DO_TEST_FULL("resctrl-skx-twocaches", VIR_ARCH_X86_64, true, true);
 DO_TEST_FULL("resctrl-fake-feature", VIR_ARCH_X86_64, true, true);
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virnuma"))
diff --git a/tests/vircapstest.c b/tests/vircapstest.c
index 9299b42bf3..8cb6fafd1d 100644
--- a/tests/vircapstest.c
+++ b/tests/vircapstest.c
@@ -249,7 +249,7 @@ mymain(void)
 ret = -1;
 #endif /* WITH_LXC */
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/virconftest.c b/tests/virconftest.c
index 52f68287d6..f1a58b01cf 100644
--- a/tests/virconftest.c
+++ b/tests/virconftest.c
@@ -465,7 +465,7 @@ mymain(void)
 if (virTestRun("string-list", testConfParseStringList, NULL) < 0)
 ret = -1;
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 
diff --git a/tests/virendiantest.c b/tests/virendiantest.c
index 38adef9353..8a6d2f5e2d 100644
--- a/tests/virendiantest.c
+++ b/tests/virendiantest.c
@@ -108,7 +108,7 @@ mymain(void)
 if (virTestRun("test2", test2, NULL) < 0)
 ret = -1;
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/virlogtest.c b/tests/virlogtest.c
index 44d71015d5..582ac5c5b8 100644
--- a/tests/virlogtest.c
+++ b/tests/virlogtest.c
@@ -151,7 +151,7 @@ mymain(void)
 TEST_PARSE_FILTERS_FAIL(":foo", 1);
 TEST_PARSE_FILTERS_FAIL("1:+", 1);
 
-return ret;
+return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 VIR_TEST_MAIN(mymain)
diff --git a/tests/virresctrltest.c b/tests/virresctrltest.c
index 00f9f552e5..26fbde3668 100644
--- a/tests/virresctrltest.c
+++ b/tests/virresctrltest.c
@@ -93,7 +93,7 @@ mymain(void)