Re: [PULL 37/48] usb: build usb-host as module

[Gerd Hoffmann]

> > +if config_host.has_key('CONFIG_USB_LIBUSB')
> 
> The problem is in this condition as it's evaluated as false. When I
> replace it with libusb.found(), everything works as it used to.

Correct fix, paolo has a pending pull request containing it.

> Unfortunately I don't know what the real goa behind using
> CONFIG_USB_LIBUSB here was to see whether my approach is good.

Used to be the correct approach before libusb detection was
switched from configure to meson.

take care,
  Gerd

Re: [PATCH] qemu: save status xml after generating taint message

[Fangge Jin]

On Wed, Jul 21, 2021 at 10:54 PM Michal Prívozník 
wrote:

>
> > The bug quoted shows a few examples where we fail to save status.
> >
> > I'm very surprised we don't save status when hotplugging a NIC or a
> > disk, as the BZ suggests.
>
> I'm not convinced that the steps there are 100% correct. We do call
> virDomainObjSave() after live attach:
>
>
> https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_driver.c#L7834
>
> The two steps for hot-unplug in the BZ actually failed.
(sorry I didn't mention the result in the steps)


> >
> > Missing status save in QMP monitor command passthrough is less
> > surprising though since we're not actually changing the VM state
> > when doing that, so would not have reason to save state except
> > for the taint message.
>
> Yep. For a few cases it is hidden in BeginJob() and EndJob() but not for
> agent jobs.
>
> Michal
>
>

Re: [libvirt PATCH] nodedev: Handle inactive mdevs with the same UUID

[Jonathon Jongsma]

On Thu, Jul 8, 2021 at 4:28 PM Jonathon Jongsma  wrote:
>
> On Thu, Jul 8, 2021 at 3:47 PM Boris Fiuczynski  wrote:
> >
> > On 7/6/21 9:05 PM, Jonathon Jongsma wrote:
> > > Unfortunately, mdevctl supports defining more than one mdev with the
> > > same UUID as long as they have different parent devices. So we can't use
> > > the UUID alone as a way to uniquely identify these node devices. Append
> > > the parent name to ensure uniqueness. For example:
> > >
> > >  Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
> > >  After:  mdev_88a6b868_46bd_4015_8e5b_26107f82da38__00_02_0
> > >
> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1979440
> > >
> > > Signed-off-by: Jonathon Jongsma 
> > > ---
> > > QUESTION: Is there any expectation of stability in these device names? 
> > > I'm not
> > > sure to what extent these changes might affect users. Given that support 
> > > for
> > > persistent mdevs is fairly new, there are likely not many users yet.
> >
> > People using mdevs with libvirt will certainly be impacted by this but I
> > am not sure how many there are.
> >
> > I am wondering if the scenario is even useful since I just tried it out
> > and starting the second mdev with the uuid fails.
> >
> > # mdevctl list -d
> > e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0033 vfio_ccw-io manual (active)
> > e60cef97-3f6b-485e-ac46-0520f9f66ac2 0.0.0034 vfio_ccw-io manual
> > # mdevctl start -u e60cef97-3f6b-485e-ac46-0520f9f66ac2 -p 0.0.0034
> > Device exists under different parent
> >
>
> Yes, it is expected that you can only have a single device with the
> same UUID active at one time. I asked Alex about why mdevctl supported
> this feature, and he said he thought it was so that a single VM xml
> could run with different mdevs depending on which one was
> instantiated. I'm not sure that anybody is actually using this
> "feature", but libvirt still has to deal with the possibility that we
> could get such a config from mdevctl.
>
> Jonathon

By the way, I'm self-NACKing this patch. I have a more comprehensive
patch series coming soon.

Jonathon

Re: [libvirt PATCH 0/4] RFC: tests: introduce lavocado

[Beraldo Leal]

On Thu, Jul 22, 2021 at 09:44:28AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 21, 2021 at 04:22:19PM -0300, Beraldo Leal wrote:
> > On Wed, Jul 21, 2021 at 06:50:03PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 01, 2021 at 06:09:47PM -0300, Beraldo Leal wrote:
> > > > On Thu, Jul 01, 2021 at 07:04:32PM +0100, Daniel P. Berrangé wrote:
> > > > > On Wed, Jun 30, 2021 at 01:36:30PM -0300, Beraldo Leal wrote:
> > > > > > I'm adding more information with some details inside the README 
> > > > > > file.
> > > > > 
> > > > > Overall, I'm more enthusiastic about writing tests in Python
> > > > > than Perl, for the long term, but would also potentially like
> > > > > to write tests in Go too.
> > > > > 
> > > > > I'm wondering if we can't bridge the divide between what we
> > > > > have already in libvirt-tck, and what you're bringing to the
> > > > > table with avocado here. While we've not done much development
> > > > > with the TCK recently, there are some very valuable tests
> > > > > there, especially related to firewall support and I don't
> > > > > fancy rewriting them.
> > > > > 
> > > > > Thus my suggestion is that we:
> > > > > 
> > > > >   - Put this avocado code into the libvirt-tck repository,
> > > > > with focus on the supporting infra for making it easy to
> > > > > write Python tests
> > > > > 
> > > > >   - Declare that all tests need a way to emit TAP format,
> > > > > no matter what language they're written in. This could
> > > > > either be the test directly emitting TAP, or it could
> > > > > be via use of a plugin. For example 'tappy' can make
> > > > > existing Python tests emit TAP, with no modifications
> > > > > to the tests themselves.
> > > > > 
> > > > >   https://tappy.readthedocs.io/en/latest/consumers.html
> > > > > 
> > > > > IOW, you can still invoke the python tests using the
> > > > > standard Python test runner, and still invoke the perl
> > > > > tests using the stnadard Perl test runner if desired.
> > > > 
> > > > This is supported already:
> > > > 
> > > > $ avocado run --tap - --test-runner='nrunner' tests/domain/transient.py
> > > > 1..3
> > > > ok 1 tests/domain/transient.py:TransientDomain.test_autostart
> > > > ok 2 tests/domain/transient.py:TransientDomain.test_lifecycle
> > > > ok 3 
> > > > tests/domain/transient.py:TransientDomain.test_convert_transient_to_persistent
> > > 
> > > This is nice, showing fine grained TAP output lines for each
> > > individual test within the test program
> > > 
> > > 
> > > I tried using the hints file that Cleber pointed to make avocado
> > > *consume* TAP format for the Perl/Shell scripts:
> > > 
> > > $ cd libvirt-tck
> > > $ cat .avocado.hint
> > > [kinds]
> > > tap = scripts/*/*.t
> > > 
> > > [tap]
> > > uri = $testpath
> > > 
> > > 
> > > And I can indeed invoke the scripts:
> > > 
> > > $ avocado run  ./scripts/domain/05*.t
> > > JOB ID : b5d596d909dc8024d986957c909fc8fb6b31e2dd
> > > JOB LOG: 
> > > /home/berrange/avocado/job-results/job-2021-07-21T18.45-b5d596d/job.log
> > >  (1/2) ./scripts/domain/050-transient-lifecycle.t: PASS (0.70 s)
> > >  (2/2) ./scripts/domain/051-transient-autostart.t: PASS (0.76 s)
> > > RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> > > CANCEL 0
> > > JOB HTML   : 
> > > /home/berrange/avocado/job-results/job-2021-07-21T18.45-b5d596d/results.html
> > > JOB TIME   : 1.90 s
> > > 
> > > which is good.
> > > 
> > > And also I can ask it to produce tap output too:
> > > 
> > > $ avocado run --tap - ./scripts/domain/05*.t
> > > 1..2
> > > ok 1 ./scripts/domain/050-transient-lifecycle.t
> > > ok 2 ./scripts/domain/051-transient-autostart.t
> > > 
> > > 
> > > But this output isn't entirely what I was after. This is just summarizing
> > > the results of each test program.
> > > 
> > > I can't find a way to make it show the fine grained tap output for the
> > > individual tests, like it does for the python program
> > 
> > Actually, the first Python TAP output example is showing a coarse TAP
> > result. I have combined three transient tests into one single file but
> > splitting them into three different methods, so we could execute each one
> > individually. i.e: tests/domain/transient.py:TransientDomain.test_autostart.
> > 
> > So, I did some reorg when migrating to Python test.
> > 
> > In order to archive the same with Perl, we could do the same there,
> > because the way individual tests are written there, doesn't allow for
> > individual execution. 
> > 
> > Yes, we could do some tricks, to parse and combine outputs and list as
> > it was a more fine graned, but afaict, we could not individually execute
> > those. This is part of Avocado test definition where in order to be
> > called a test, we need to be able to execute those individually as well.
> 
> Ok, I'm not so fussed about whether avocado can ultimately preserve the
> fine grained TAP output. Mostly I'm looking to understand how you should
> debug 

Re: [libvirt PATCH 0/4] RFC: tests: introduce lavocado

[Cleber Rosa]

Daniel P. Berrangé writes:

> On Tue, Jul 20, 2021 at 02:19:25PM +0200, Erik Skultety wrote:
>> On Thu, Jul 01, 2021 at 07:04:32PM +0100, Daniel P. Berrangé wrote:
>> > Libvirt has consumers writing applications in a variety of
>> > languages, and periodically reporting bugs.  My general wish
>> > for a test harness would be for something that can invoke
>> > and consume arbitrary test cases. Essentially if someone
>> > can provide us a piece of code that demonstrates a problem
>> > in libvirt, I would like to be able to use that directly as
>> > a functional test, regardless of language it was written
>> > in.
>> 
>> Well, in theory yes - it is all nice to let anyone contribute tests in their
>> language of preference. Realistically, look at the nwfilter stuff in TCK, 
>> it's
>> a pile of unintelligible Bash which cannot be debugged. So, I understand why
>> you'd want to imagine or write tests in Go, but someone will have to maintain
>> them otherwise we'd end up with a 3rd framework nobody will want to use.
>
> The nwfilter shell script was just badly written, and we should have
> pushed back on accepting it in that state, but it is was so broadly
> functional it was preferrable than getting no testing. It could have
> been just as horrible if written in Perl or Python too - the core
> approach of the tests themselves was just wrong. The use of shell
> was just icing on the cake.
>

Clearly that are exceptions, but I agree that for non-unit tests, the
"foreign" aspect of a language is pretty much irrelevant.  The
suitability of a given language will mostly be based on its own
characteristics, and Python seems to do quite well for writing tests
indeed.

Also, my impression is that the quality of documentation, test templates
and relevant functionality that is found in an existing "framework" is
generally inversely proportional to people's motivation to write a test
in another language.  Even if people have another language they strongly
prefer, and even if the project will execute without tests written in other
languages without major hurdles, documentation/templates/relevant APIs
weight very heavily on the other side of the scale.

That explains at least partially why Perl is, still to this day, the
language of virtually every single test on TCK.

>
> The main point is that we have applications consuming libvirt in
> multiple languages. Python and Go are the most important ones in
> terms of usage today, and we get some quite detailed bug reports
> with demo code. If we get a bug report with a Go demo program,
> it can't be assumed to be easy to just convert that to Python,
> and vica-verca.
>
> We do have to consider the maintainability of any tests being
> contributed, but I don't want to be rejecting bug reproducers
> that work, just because they're not in a specific language.
>
>
>

QEMU is a close example of tests written in many languages.  I agree
that it's better to have a test in whatever language than none at all.

Having said that, maintaining the quality of tests over time is hard
enough when they are mostly uniform (built and/or executed similarly).
When tests are written in "one off" languages that require extra steps
to be built/executed, this is further aggravated.

In short, as long as there's an easy way to run tests, they are run
often, results are clear and logs are also somewhat uniform (if not in
content at least wrt location), then it's a no-brainer whether to keep a
contributed test or not in the tree.

>> Luckily, Python as a language doesn't suffer from this and we've already made
>> that preference in the libvirt repo already. So, I'd suggest to use our
>> expertise in helping contributors who wish to contribute a test with porting
>> their test case to a Python one - yes, it does consumer human resources and
>> prolongs the time for a contribution to be accepted, but that pays off later
>> down the road when the test cases need to be adapted to new
>> conditions/environments.
>
> Certainly I'd say we can consider python as the default or preferred
> language for writing tests, but I'd like us to be open to accepting
> tests in other languages. 
>
>
>> > In theory the libvirt TCK allowed for that, and indeed the
>> > few tests written in shell essentially came to exist because
>> > someone at IBM had written some shell code to test firewalls
>> > and contributed that as a functional test. They just hacked
>> > their script to emit TAP data.
>> 
>> Others have already responded that Avocado can already do the same.
>> What I'd like to say, again in context of arbitrary languages used in tests,
>> the problem with the whole bash machinery IBM contributed is not only that it
>> cannot be debugged, but also that you cannot replay a single failed test, so 
>> if
>> an error happened in e.g. test 1148, you have literally no idea how to work
>> only with that test case. Knowing literally zero about Avocado's nrunner and
>> external (non-native) test suite support I cannot tell whet

Re: set custom loglevel for external libraries

[Olaf Hering]

Am Thu, 22 Jul 2021 17:17:11 +0200
schrieb Olaf Hering :

> xc_private.h uses XTL_INFO, XTL_DETAIL, XTL_DEBUG and XTL_ERROR in the 
> wrappers.

I wonder if the code should map VIR_LOG_INFO to XTL_DETAIL to enable DPRINTF.

Olaf


pgp3lN57hElrs.pgp
Description: Digitale Signatur von OpenPGP

Re: [libvirt PATCH] ci: Adapt to lcitool command line changes

[Erik Skultety]

On Thu, Jul 22, 2021 at 04:01:13PM +0200, Andrea Bolognani wrote:
> lcitool now uses the term "target" instead of "host" to refer to
> the various operating systems it supports, and we need to adapt
> our helper script so that it works with the new command line
> interface.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> Getting this ready for when
> 
>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/92
> 
> is merged.

You can use:
Reviewed-by: Erik Skultety 

when it's merged

Re: [PATCH v3] nodedev: fix internal error when no defined mdevs exist

[Jonathon Jongsma]

On Wed, 21 Jul 2021 13:30:00 +0200
Boris Fiuczynski  wrote:

> Commit e9b534905f4 introduced an error when parsing an empty list
> returned from mdevctl.
> 
> This occurs e.g. if nodedev-undefine is used to undefine the last
> defined mdev which causes the following error messages
> 
>  libvirtd[33143]: internal error: Unexpected format for mdevctl
> response libvirtd[33143]: internal error: failed to query mdevs from
> mdevctl: libvirtd[33143]: mdevctl failed to updated mediated devices
> 
> Signed-off-by: Boris Fiuczynski 
> Reviewed-by: Shalini Chellathurai Saroja 
> Reviewed-by: Martin Kletzander 
> ---
>  src/node_device/node_device_driver.c| 6 ++
>  tests/nodedevmdevctldata/mdevctl-list-empty.json| 1 +
>  tests/nodedevmdevctldata/mdevctl-list-empty.out.xml | 0
>  tests/nodedevmdevctltest.c  | 1 +
>  4 files changed, 8 insertions(+)
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json
>  create mode 100644
> tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
> 
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c index 17a7aea2bb..6f8968f1f0
> 100644 --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1139,6 +1139,12 @@ nodeDeviceParseMdevctlJSON(const char
> *jsonstring, goto error;
>  }
>  
> +if (virJSONValueArraySize(json_devicelist) == 0) {
> +VIR_DEBUG("mdevctl has no defined mediated devices");
> +*devs = NULL;
> +return 0;
> +}
> +
>  /* mdevctl list --dumpjson produces an output that is an array
> that
>   * contains only a single object which contains a property for
> each parent
>   * device */
> diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json
> b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode
> 100644 index 00..fe51488c70
> --- /dev/null
> +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json
> @@ -0,0 +1 @@
> +[]
> diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml
> b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode
> 100644 index 00..e69de29bb2
> diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
> index 8ba1d2da70..e246de4d87 100644
> --- a/tests/nodedevmdevctltest.c
> +++ b/tests/nodedevmdevctltest.c
> @@ -360,6 +360,7 @@ mymain(void)
>  
>  DO_TEST_LIST_DEFINED();
>  
> +DO_TEST_PARSE_JSON("mdevctl-list-empty");
>  DO_TEST_PARSE_JSON("mdevctl-list-multiple");
>  
>  DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366");



Reviewed-by: Jonathon Jongsma 

I've pushed it upstream.

Thanks,
Jonathon

[PATCH 2/2] virSetUIDGIDWithCaps: Set bounding capabilities only with CAP_SETPCAP

[Michal Privoznik]

In one of my previous patches I've tried to postpone dropping
CAP_SETPCAP until the very end because it's needed for
capng_apply(). What I did not realize back then was that we might
not have the capability to begin with. Because of unknown reasons
capng_apply() pollutes logs only for CAPNG_SELECT_BOUNDS and not
for CAPNG_SELECT_CAPS.

Reproducer is really simple: run libvirtd as a regular user.
During its initialization, libvirtd will spawn some binaries
(dnsmasq, qemu-*, etc.) and while doing so it will try to drop
capabilities.

Anyway, let's call capng_apply(CAPNG_SELECT_BOUNDS) only if we
have the CAP_SETPCAP (which is tracked in need_setpcap variable).

Fixes: 438b50dda8a863fdc988e9ab612f097cc1626e8a
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1924218
Signed-off-by: Michal Privoznik 
---
 src/util/virutil.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index aba0aea0ff..00cd56e2b2 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1250,7 +1250,8 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, 
int ngroups,
  * do this if we failed to get the capability above, so ignore the
  * return value.
  */
-capng_apply(CAPNG_SELECT_BOUNDS);
+if (!need_setpcap)
+capng_apply(CAPNG_SELECT_BOUNDS);
 
 /* Drop the caps that allow setuid/gid (unless they were requested) */
 if (need_setgid)
-- 
2.31.1

[PATCH 0/2] Another round of capng_apply() fixes

[Michal Privoznik]

See 2/2 for explanation.

Michal Prívozník (2):
  virSetUIDGIDWithCaps: Check for capng_apply() retval properly
  virSetUIDGIDWithCaps: Set bounding capabilities only with CAP_SETPCAP

 src/util/virutil.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.31.1

[PATCH 1/2] virSetUIDGIDWithCaps: Check for capng_apply() retval properly

[Michal Privoznik]

After all capabilities were set (except for CAP_SETGID,
CAP_SETUID and CAP_SETPCAP) and after UID:GID was changed we drop
the last aforementioned capabilities (we couldn't drop them
before because we needed UID:GID and capabilities change).
Therefore, there's final capng_apply() call. However, it's return
value is not checked for properly. It's typical problem of:

  var = func() < 0

Signed-off-by: Michal Privoznik 
---
 src/util/virutil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index ed3d57662b..aba0aea0ff 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1261,7 +1261,7 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, 
int ngroups,
 if (need_setpcap)
 capng_update(CAPNG_DROP, CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETPCAP);
 
-if (((capng_ret = capng_apply(CAPNG_SELECT_CAPS)) < 0)) {
+if ((capng_ret = capng_apply(CAPNG_SELECT_CAPS)) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot apply process capabilities %d"), capng_ret);
 return -1;
-- 
2.31.1

Re: set custom loglevel for external libraries

[Olaf Hering]

Am Thu, 22 Jul 2021 16:07:15 +0100
schrieb Daniel P. Berrangé :

> Do we actually need to care about every single level, as opposed to
> just doing a sparse mapping where we squash several xen levels into
> one libvirt level, as the code below shows ?

xc_private.h uses XTL_INFO, XTL_DETAIL, XTL_DEBUG and XTL_ERROR in the wrappers.

I think in the end all of the values can be used once execution enters 
libxenlight.so.
The code I changed had to map the few libvirt values somehow to the XTL values.


Olaf


pgpCTuKzh12Ay.pgp
Description: Digitale Signatur von OpenPGP

Re: set custom loglevel for external libraries

[Daniel P . Berrangé]

On Thu, Jul 22, 2021 at 04:39:24PM +0200, Olaf Hering wrote:
> Thanks for your reply, I had no time to look at this until now.
> 
> Am Wed, 16 Jun 2021 11:44:10 +0100
> schrieb Daniel P. Berrangé :
> 
> > Anyway if you used a virLogSource object, then turning on debugging
> > exclusively for libxl library messages would be as simple as
> > 
> >   log_filters="1:libxl.libxl_library"
> > 
> > without affecting the global libvirt logging behaviour
> 
> It seems log_filters= does not allow arbitrary numbers.
> virLogParseFilter would just allow values from 1 to 4.
> Such extra logfilter needs to handle a xentoollog_level, values from 1 to 9 
> (>XTL_NONE and  
> How should this be expressed in a config setting?

Do we actually need to care about every single level, as opposed to
just doing a sparse mapping where we squash several xen levels into
one libvirt level, as the code below shows ?

> 
> Perhaps just go the easy route and obtain a value from environment?
> 
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1829,7 +1829,7 @@ libxlDriverConfigInit(libxlDriverConfig *cfg)
>  return -1;
>  }
>  
> -cfg->logger = libxlLoggerNew(cfg->logDir, virLogGetDefaultPriority());
> +cfg->logger = libxlLoggerNew(cfg->logDir);
>  if (!cfg->logger) {
>  VIR_ERROR(_("cannot create logger for libxenlight, disabling 
> driver"));
>  return -1;
> --- a/src/libxl/libxl_logger.c
> +++ b/src/libxl/libxl_logger.c
> @@ -129,25 +129,33 @@ libvirt_destroy(xentoollog_logger *logger_in)
>  
>  
>  libxlLogger *
> -libxlLoggerNew(const char *logDir, virLogPriority minLevel)
> +libxlLoggerNew(const char *logDir)
>  {
>  xentoollog_logger_libvirt logger;
>  g_autofree char *path = NULL;
> -
> -switch (minLevel) {
> -case VIR_LOG_DEBUG:
> -logger.minLevel = XTL_DEBUG;
> -break;
> -case VIR_LOG_INFO:
> -logger.minLevel = XTL_INFO;
> -break;
> -case VIR_LOG_WARN:
> -logger.minLevel = XTL_WARN;
> -break;
> -case VIR_LOG_ERROR:
> -logger.minLevel = XTL_ERROR;
> -break;
> +char *xenlight_minlevel = getenv("xenlight_minlevel");
> +int minlevel;
> +
> +if (!(xenlight_minlevel && *xenlight_minlevel &&
> +virStrToLong_i(xenlight_minlevel, NULL, 10, &minlevel) >= 0 &&
> +minlevel > XTL_NONE && minlevel < XTL_NUM_LEVELS)) {
> +minlevel = virLogGetDefaultPriority();
> +switch (minlevel) {
> +case VIR_LOG_DEBUG:
> +minlevel = XTL_DEBUG;
> +break;
> +case VIR_LOG_INFO:
> +minlevel = XTL_INFO;
> +break;
> +case VIR_LOG_WARN:
> +minlevel = XTL_WARN;
> +break;
> +case VIR_LOG_ERROR:
> +minlevel = XTL_ERROR;
> +break;
> +}
>  }
> +logger.minLevel = minlevel;
>  logger.logDir = logDir;
>  
>  if ((logger.files = virHashNew(libxlLoggerFileFree)) == NULL)
> --- a/src/libxl/libxl_logger.h
> +++ b/src/libxl/libxl_logger.h
> @@ -24,8 +24,7 @@
>  
>  typedef struct xentoollog_logger_libvirt libxlLogger;
>  
> -libxlLogger *libxlLoggerNew(const char *logDir,
> -  virLogPriority minLevel);
> +libxlLogger *libxlLoggerNew(const char *logDir);
>  void libxlLoggerFree(libxlLogger *logger);
>  
>  void libxlLoggerOpenFile(libxlLogger *logger, int id, const char *name,
> 
> 
> Olaf



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: [PULL 36/40] vl: switch -M parsing to keyval

[Peter Krempa]

On Thu, Jul 22, 2021 at 16:39:26 +0200, Paolo Bonzini wrote:
> On 22/07/21 10:19, Peter Krempa wrote:
> > This patch breaks detection of certain machine options features in
> > libvirt which were being detected from 'query-command-line-options'.
> > 
> > I presume the change simply removed this from the output of
> > query-command-line-options due to the historical cruft how the
> > aforementioned command works.
> > 
> > Unfortunately I didn't find any suitable replacement from what we are
> > querying.
> 
> Yep, there is already a patch queued for this.

So the desired state is that the properties stay in
'query-command-line-options'?

Re: set custom loglevel for external libraries

[Olaf Hering]

Thanks for your reply, I had no time to look at this until now.

Am Wed, 16 Jun 2021 11:44:10 +0100
schrieb Daniel P. Berrangé :

> Anyway if you used a virLogSource object, then turning on debugging
> exclusively for libxl library messages would be as simple as
> 
>   log_filters="1:libxl.libxl_library"
> 
> without affecting the global libvirt logging behaviour

It seems log_filters= does not allow arbitrary numbers.
virLogParseFilter would just allow values from 1 to 4.
Such extra logfilter needs to handle a xentoollog_level, values from 1 to 9 
(>XTL_NONE and logger = libxlLoggerNew(cfg->logDir, virLogGetDefaultPriority());
+cfg->logger = libxlLoggerNew(cfg->logDir);
 if (!cfg->logger) {
 VIR_ERROR(_("cannot create logger for libxenlight, disabling driver"));
 return -1;
--- a/src/libxl/libxl_logger.c
+++ b/src/libxl/libxl_logger.c
@@ -129,25 +129,33 @@ libvirt_destroy(xentoollog_logger *logger_in)
 
 
 libxlLogger *
-libxlLoggerNew(const char *logDir, virLogPriority minLevel)
+libxlLoggerNew(const char *logDir)
 {
 xentoollog_logger_libvirt logger;
 g_autofree char *path = NULL;
-
-switch (minLevel) {
-case VIR_LOG_DEBUG:
-logger.minLevel = XTL_DEBUG;
-break;
-case VIR_LOG_INFO:
-logger.minLevel = XTL_INFO;
-break;
-case VIR_LOG_WARN:
-logger.minLevel = XTL_WARN;
-break;
-case VIR_LOG_ERROR:
-logger.minLevel = XTL_ERROR;
-break;
+char *xenlight_minlevel = getenv("xenlight_minlevel");
+int minlevel;
+
+if (!(xenlight_minlevel && *xenlight_minlevel &&
+virStrToLong_i(xenlight_minlevel, NULL, 10, &minlevel) >= 0 &&
+minlevel > XTL_NONE && minlevel < XTL_NUM_LEVELS)) {
+minlevel = virLogGetDefaultPriority();
+switch (minlevel) {
+case VIR_LOG_DEBUG:
+minlevel = XTL_DEBUG;
+break;
+case VIR_LOG_INFO:
+minlevel = XTL_INFO;
+break;
+case VIR_LOG_WARN:
+minlevel = XTL_WARN;
+break;
+case VIR_LOG_ERROR:
+minlevel = XTL_ERROR;
+break;
+}
 }
+logger.minLevel = minlevel;
 logger.logDir = logDir;
 
 if ((logger.files = virHashNew(libxlLoggerFileFree)) == NULL)
--- a/src/libxl/libxl_logger.h
+++ b/src/libxl/libxl_logger.h
@@ -24,8 +24,7 @@
 
 typedef struct xentoollog_logger_libvirt libxlLogger;
 
-libxlLogger *libxlLoggerNew(const char *logDir,
-  virLogPriority minLevel);
+libxlLogger *libxlLoggerNew(const char *logDir);
 void libxlLoggerFree(libxlLogger *logger);
 
 void libxlLoggerOpenFile(libxlLogger *logger, int id, const char *name,


Olaf


pgpcXcOTKztMi.pgp
Description: Digitale Signatur von OpenPGP

Re: [PULL 36/40] vl: switch -M parsing to keyval

[Paolo Bonzini]

On 22/07/21 10:19, Peter Krempa wrote:

This patch breaks detection of certain machine options features in
libvirt which were being detected from 'query-command-line-options'.

I presume the change simply removed this from the output of
query-command-line-options due to the historical cruft how the
aforementioned command works.

Unfortunately I didn't find any suitable replacement from what we are
querying.


Yep, there is already a patch queued for this.

Paolo

Re: [PULL 37/48] usb: build usb-host as module

[Peter Krempa]

adding libvirt-list

On Thu, Jul 08, 2021 at 17:17:37 +0200, Paolo Bonzini wrote:
> From: Gerd Hoffmann 
> 
> Drop one more shared library dependency (libusb) from core qemu.
> 
> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Jose R. Ziviani 
> Message-Id: <20210624103836.2382472-34-kra...@redhat.com>
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/usb/host-libusb.c | 1 +
>  hw/usb/meson.build   | 8 ++--
>  2 files changed, 7 insertions(+), 2 deletions(-)

After this commit libvirt is no longer detecting the 'hostdevice'
property of 'usb-host'. In fact 'device-list-properties' is returning:

"desc": "Device 'usb-host' not found"

> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index 2b7f87872c..c0f314462a 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -1777,6 +1777,7 @@ static TypeInfo usb_host_dev_info = {
>  .class_init= usb_host_class_initfn,
>  .instance_init = usb_host_instance_init,
>  };
> +module_obj(TYPE_USB_HOST_DEVICE);
>  
>  static void usb_host_register_types(void)
>  {
> diff --git a/hw/usb/meson.build b/hw/usb/meson.build
> index 817f3e027a..3ca6127937 100644
> --- a/hw/usb/meson.build
> +++ b/hw/usb/meson.build
> @@ -72,8 +72,12 @@ if usbredir.found()
>  endif
>  
>  # usb pass-through
> -softmmu_ss.add(when: ['CONFIG_USB', libusb],
> -   if_true: files('host-libusb.c'))
> +if config_host.has_key('CONFIG_USB_LIBUSB')

The problem is in this condition as it's evaluated as false. When I
replace it with libusb.found(), everything works as it used to.

Unfortunately I don't know what the real goa behind using
CONFIG_USB_LIBUSB here was to see whether my approach is good.

> +  usbhost_ss = ss.source_set()
> +  usbhost_ss.add(when: ['CONFIG_USB', libusb],
> + if_true: files('host-libusb.c'))
> +  hw_usb_modules += {'host': usbhost_ss}
> +endif
>  
>  softmmu_ss.add(when: ['CONFIG_USB', 'CONFIG_XEN', libusb], if_true: 
> files('xen-usb.c'))
>  
> -- 
> 2.31.1
> 
> 
> 

[libvirt PATCH] ci: Adapt to lcitool command line changes

[Andrea Bolognani]

lcitool now uses the term "target" instead of "host" to refer to
the various operating systems it supports, and we need to adapt
our helper script so that it works with the new command line
interface.

Signed-off-by: Andrea Bolognani 
---
Getting this ready for when

  https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/92

is merged.

 ci/helper | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/ci/helper b/ci/helper
index 2c3a9f8db4..441258f511 100755
--- a/ci/helper
+++ b/ci/helper
@@ -190,18 +190,18 @@ class Application:
 output = subprocess.check_output([self._args.lcitool] + args)
 return output.decode("utf-8")
 
-def _lcitool_get_hosts(self):
-output = self._lcitool_run(["hosts"])
+def _lcitool_get_targets(self):
+output = self._lcitool_run(["targets"])
 return output.splitlines()
 
-def _generate_dockerfile(self, host, cross=None):
-args = ["dockerfile", host, "libvirt"]
+def _generate_dockerfile(self, target, cross=None):
+args = ["dockerfile", target, "libvirt"]
 outdir = self._basedir.joinpath("containers")
-outfile = f"{host}.Dockerfile"
+outfile = f"{target}.Dockerfile"
 
 if cross:
 args.extend(["--cross", cross])
-outfile = f"{host}-cross-{cross}.Dockerfile"
+outfile = f"{target}-cross-{cross}.Dockerfile"
 
 outpath = outdir.joinpath(outfile)
 if not self._args.quiet:
@@ -211,10 +211,10 @@ class Application:
 with open(outpath, "w") as f:
 f.write(output)
 
-def _generate_vars(self, host):
-args = ["variables", host, "libvirt"]
+def _generate_vars(self, target):
+args = ["variables", target, "libvirt"]
 outdir = self._basedir.joinpath("cirrus")
-outfile = f"{host}.vars"
+outfile = f"{target}.vars"
 
 outpath = outdir.joinpath(outfile)
 if not self._args.quiet:
@@ -241,37 +241,37 @@ class Application:
 "mingw64",
 ]
 
-for host in self._lcitool_get_hosts():
-if host.startswith("freebsd-") or host.startswith("macos-"):
+for target in self._lcitool_get_targets():
+if target.startswith("freebsd-") or target.startswith("macos-"):
 continue
 
-self._generate_dockerfile(host)
+self._generate_dockerfile(target)
 
-if host == "fedora-rawhide":
+if target == "fedora-rawhide":
 for cross in fedora_cross:
-self._generate_dockerfile(host, cross)
+self._generate_dockerfile(target, cross)
 
-if host.startswith("debian-"):
+if target.startswith("debian-"):
 for cross in debian_cross:
-if host == "debian-sid" and cross == "mips":
+if target == "debian-sid" and cross == "mips":
 continue
-self._generate_dockerfile(host, cross)
+self._generate_dockerfile(target, cross)
 
 def _refresh_cirrus(self):
-for host in self._lcitool_get_hosts():
-if not (host.startswith("freebsd-") or host.startswith("macos-")):
+for target in self._lcitool_get_targets():
+if not (target.startswith("freebsd-") or 
target.startswith("macos-")):
 continue
 
-self._generate_vars(host)
+self._generate_vars(target)
 
 def _check_stale_images(self):
 namespace = self._args.namespace
 gitlab_uri = self._args.gitlab_uri
 registry_uri = util.get_registry_uri(namespace, gitlab_uri)
-lcitool_hosts = self._lcitool_get_hosts()
+lcitool_targets = self._lcitool_get_targets()
 
 stale_images = util.get_registry_stale_images(registry_uri,
-  lcitool_hosts)
+  lcitool_targets)
 if stale_images:
 spacing = "\n" + 4 * " "
 stale_fmt = [f"{k} (ID: {v})" for k, v in stale_images.items()]
-- 
2.31.1

Re: [PATCH] qemuxml2argvmock: Deduplicate some code

[Jano Tomko]

On a %A in %Y, Michal Privoznik wrote:
> A stub implementation for virQEMUCapsGetKVMSupportsSecureGuest()
> was added recently. However, the very same implementation was
> added to domaincapsmock.c which is also loaded by
> qemuxml2argvtest and thus one occurrence of the stub can be
> dropped.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  tests/qemuxml2argvmock.c | 15 ---
>  1 file changed, 15 deletions(-)
> 
Reviewed-by: Ján Tomko 

Jano

Re: [libvirt PATCH] qemuxml2argvmock: drop virQEMUCapsGetKVMSupportsSecureGuest

[Michal Prívozník]

On 7/22/21 3:17 PM, Pavel Hrdina wrote:
> It is actually not needed because in qemuxml2argvtest we preload
> domaincapsmock as well.
> 
> Reported-by: Michal Privoznik 
> Signed-off-by: Pavel Hrdina 
> ---
>  tests/qemuxml2argvmock.c | 16 
>  1 file changed, 16 deletions(-)
> 

D'oh! I've just sent the same patch. I had it ready but was waiting for
my perl packages to upgrade, because git-send-email was unhappy about
openssl lib version mismatch :-)

Reviewed-by: Michal Privoznik 

Michal

[PATCH] qemuxml2argvmock: Deduplicate some code

[Michal Privoznik]

A stub implementation for virQEMUCapsGetKVMSupportsSecureGuest()
was added recently. However, the very same implementation was
added to domaincapsmock.c which is also loaded by
qemuxml2argvtest and thus one occurrence of the stub can be
dropped.

Signed-off-by: Michal Privoznik 
---
 tests/qemuxml2argvmock.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index d7f77eabf7..9235c785b2 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -302,18 +302,3 @@ virIdentityEnsureSystemToken(void)
 {
 return g_strdup("3de80bcbf22d4833897f1638e01be9b2");
 }
-
-static bool (*real_virQEMUCapsGetKVMSupportsSecureGuest)(virQEMUCaps 
*qemuCaps);
-
-bool
-virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps)
-{
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) 
&&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_S390_PV_GUEST))
-return true;
-
-if (!real_virQEMUCapsGetKVMSupportsSecureGuest)
-VIR_MOCK_REAL_INIT(virQEMUCapsGetKVMSupportsSecureGuest);
-
-return real_virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps);
-}
-- 
2.31.1

[PATCH v2 2/3] remove sysconfig files

[Olaf Hering]

sysconfig files are owned by the admin of the host. They have the
liberty to put anything they want into these files. This makes it
difficult to provide different built-in defaults.

Remove the sysconfig file and place the current desired default into
the service file.

Local customizations can now go either into /etc/sysconfig/name
or /etc/systemd/system/name.service.d/my-knobs.conf

Attempt to handle upgrades in libvirt.spec.
Dirty files which are marked as %config will be renamed to file.rpmsave.
To restore them automatically, move stale .rpmsave files away, and
catch any new rpmsave files in %posttrans.

Signed-off-by: Olaf Hering 
---
v2:
- fix libvirt_sc_pre usage
- leave existing grep in sysconfig in %posttrans daemon as it is
- leave existing %post daemon as it is
- add empty VIRTLOCKD_ARGS= to virtlockd.service
- provide drop-in example in daemons.rst
---
 docs/daemons.rst| 20 
 docs/remote.html.in |  2 +-
 libvirt.spec.in | 68 +++--
 src/ch/meson.build  |  5 --
 src/ch/virtchd.service.in   |  1 +
 src/ch/virtchd.sysconf  |  3 --
 src/interface/meson.build   |  5 --
 src/interface/virtinterfaced.service.in |  1 +
 src/interface/virtinterfaced.sysconf|  3 --
 src/libxl/meson.build   |  5 --
 src/libxl/virtxend.service.in   |  1 +
 src/libxl/virtxend.sysconf  |  3 --
 src/locking/meson.build |  5 --
 src/locking/virtlockd.service.in|  1 +
 src/locking/virtlockd.sysconf   |  3 --
 src/logging/meson.build |  5 --
 src/logging/virtlogd.sysconf|  3 --
 src/lxc/meson.build |  5 --
 src/lxc/virtlxcd.service.in |  1 +
 src/lxc/virtlxcd.sysconf|  3 --
 src/meson.build | 16 --
 src/network/meson.build |  5 --
 src/network/virtnetworkd.service.in |  1 +
 src/network/virtnetworkd.sysconf|  3 --
 src/node_device/meson.build |  5 --
 src/node_device/virtnodedevd.service.in |  1 +
 src/node_device/virtnodedevd.sysconf|  3 --
 src/nwfilter/meson.build|  5 --
 src/nwfilter/virtnwfilterd.service.in   |  1 +
 src/nwfilter/virtnwfilterd.sysconf  |  3 --
 src/qemu/meson.build|  5 --
 src/qemu/virtqemud.service.in   |  7 +++
 src/qemu/virtqemud.sysconf  | 12 -
 src/remote/libvirtd.service.in  |  7 +++
 src/remote/libvirtd.sysconf | 21 
 src/remote/meson.build  | 10 
 src/remote/virtproxyd.service.in|  1 +
 src/remote/virtproxyd.sysconf   |  3 --
 src/secret/meson.build  |  5 --
 src/secret/virtsecretd.service.in   |  1 +
 src/secret/virtsecretd.sysconf  |  3 --
 src/storage/meson.build |  5 --
 src/storage/virtstoraged.service.in |  1 +
 src/storage/virtstoraged.sysconf|  3 --
 src/vbox/meson.build|  5 --
 src/vbox/virtvboxd.service.in   |  1 +
 src/vbox/virtvboxd.sysconf  |  3 --
 src/vz/meson.build  |  5 --
 src/vz/virtvzd.service.in   |  1 +
 src/vz/virtvzd.sysconf  |  3 --
 tools/libvirt-guests.sh.in  | 40 +++
 tools/libvirt-guests.sysconf| 50 --
 tools/meson.build   |  6 ---
 53 files changed, 141 insertions(+), 243 deletions(-)
 delete mode 100644 src/ch/virtchd.sysconf
 delete mode 100644 src/interface/virtinterfaced.sysconf
 delete mode 100644 src/libxl/virtxend.sysconf
 delete mode 100644 src/locking/virtlockd.sysconf
 delete mode 100644 src/logging/virtlogd.sysconf
 delete mode 100644 src/lxc/virtlxcd.sysconf
 delete mode 100644 src/network/virtnetworkd.sysconf
 delete mode 100644 src/node_device/virtnodedevd.sysconf
 delete mode 100644 src/nwfilter/virtnwfilterd.sysconf
 delete mode 100644 src/qemu/virtqemud.sysconf
 delete mode 100644 src/remote/libvirtd.sysconf
 delete mode 100644 src/remote/virtproxyd.sysconf
 delete mode 100644 src/secret/virtsecretd.sysconf
 delete mode 100644 src/storage/virtstoraged.sysconf
 delete mode 100644 src/vbox/virtvboxd.sysconf
 delete mode 100644 src/vz/virtvzd.sysconf
 delete mode 100644 tools/libvirt-guests.sysconf

diff --git a/docs/daemons.rst b/docs/daemons.rst
index 8a05ee77a7..1b6396d2af 100644
--- a/docs/daemons.rst
+++ b/docs/daemons.rst
@@ -686,3 +686,23 @@ socket unit names into the service. When using these old 
versions, the
 ``unix_sock_dir`` setting in ``virtlockd.conf`` must be changed in
 lock-step with the equivalent setting in the unit files to ensure that
 ``virtlockd`` can identify the sockets.
+
+Changing command line options for daemons
+=
+
+Two ways exist to override the defaults in the provided service fi

[PATCH v2 3/3] NEWS: mention removal of sysconfig

[Olaf Hering]

Signed-off-by: Olaf Hering 
---
 NEWS.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index d95750e776..3e5b790e03 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -15,6 +15,16 @@ v7.6.0 (unreleased)
 
 * **Improvements**
 
+  * packaging: sysconfig files no longer installed
+
+libvirt used to provide defaults in various /etc/sysconfig/ files, such
+as /etc/sysconfig/libvirt. Since these files are owned by the admin, this
+made it difficult to change built-in defaults in case such file was
+modified by the admin. The built-in defaults are now part of the provided
+systemd unit files, such as libvirtd.service. These unit files continue
+to parse sysconfig files, in case they are created by the admin and filled
+with the desired key=value pairs.
+
 * **Bug fixes**
 
   * qemu: Fix migration with VIR_MIGRATE_NON_SHARED_INC

[PATCH v2 1/3] libvirt.spec: relocate pre script of daemon-driver-qemu

[Olaf Hering]

Reduce the delta in an upcoming change.
No change in behavior intended.

Signed-off-by: Olaf Hering 
---
 libvirt.spec.in | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index cb48dd0be0..85e79883c5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1417,23 +1417,6 @@ fi
 rm -rf %{_localstatedir}/lib/rpm-state/libvirt || :
 
 
-%if %{with_qemu}
-%pre daemon-driver-qemu
-# We want soft static allocation of well-known ids, as disk images
-# are commonly shared across NFS mounts by id rather than name; see
-# https://fedoraproject.org/wiki/Packaging:UsersAndGroups
-getent group kvm >/dev/null || groupadd -f -g 36 -r kvm
-getent group qemu >/dev/null || groupadd -f -g 107 -r qemu
-if ! getent passwd qemu >/dev/null; then
-  if ! getent passwd 107 >/dev/null; then
-useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu
-  else
-useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu
-  fi
-fi
-exit 0
-%endif
-
 %if %{with_lxc}
 %pre login-shell
 getent group virtlogin >/dev/null || groupadd -r virtlogin
@@ -1688,6 +1671,21 @@ exit 0
 %endif
 
 %if %{with_qemu}
+%pre daemon-driver-qemu
+# We want soft static allocation of well-known ids, as disk images
+# are commonly shared across NFS mounts by id rather than name; see
+# https://fedoraproject.org/wiki/Packaging:UsersAndGroups
+getent group kvm >/dev/null || groupadd -f -g 36 -r kvm
+getent group qemu >/dev/null || groupadd -f -g 107 -r qemu
+if ! getent passwd qemu >/dev/null; then
+  if ! getent passwd 107 >/dev/null; then
+useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu
+  else
+useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu
+  fi
+fi
+exit 0
+
 %files daemon-driver-qemu
 %config(noreplace) %{_sysconfdir}/sysconfig/virtqemud
 %config(noreplace) %{_sysconfdir}/libvirt/virtqemud.conf

[PATCH v2 0/3] remove sysconfig files

[Olaf Hering]

based on 94944e38d7.

Olaf Hering (3):
  libvirt.spec: relocate pre script of daemon-driver-qemu
  remove sysconfig files
  NEWS: mention removal of sysconfig

 NEWS.rst|  10 +++
 docs/daemons.rst|  20 +
 docs/remote.html.in |   2 +-
 libvirt.spec.in | 100 
 src/ch/meson.build  |   5 --
 src/ch/virtchd.service.in   |   1 +
 src/ch/virtchd.sysconf  |   3 -
 src/interface/meson.build   |   5 --
 src/interface/virtinterfaced.service.in |   1 +
 src/interface/virtinterfaced.sysconf|   3 -
 src/libxl/meson.build   |   5 --
 src/libxl/virtxend.service.in   |   1 +
 src/libxl/virtxend.sysconf  |   3 -
 src/locking/meson.build |   5 --
 src/locking/virtlockd.service.in|   1 +
 src/locking/virtlockd.sysconf   |   3 -
 src/logging/meson.build |   5 --
 src/logging/virtlogd.sysconf|   3 -
 src/lxc/meson.build |   5 --
 src/lxc/virtlxcd.service.in |   1 +
 src/lxc/virtlxcd.sysconf|   3 -
 src/meson.build |  16 
 src/network/meson.build |   5 --
 src/network/virtnetworkd.service.in |   1 +
 src/network/virtnetworkd.sysconf|   3 -
 src/node_device/meson.build |   5 --
 src/node_device/virtnodedevd.service.in |   1 +
 src/node_device/virtnodedevd.sysconf|   3 -
 src/nwfilter/meson.build|   5 --
 src/nwfilter/virtnwfilterd.service.in   |   1 +
 src/nwfilter/virtnwfilterd.sysconf  |   3 -
 src/qemu/meson.build|   5 --
 src/qemu/virtqemud.service.in   |   7 ++
 src/qemu/virtqemud.sysconf  |  12 ---
 src/remote/libvirtd.service.in  |   7 ++
 src/remote/libvirtd.sysconf |  21 -
 src/remote/meson.build  |  10 ---
 src/remote/virtproxyd.service.in|   1 +
 src/remote/virtproxyd.sysconf   |   3 -
 src/secret/meson.build  |   5 --
 src/secret/virtsecretd.service.in   |   1 +
 src/secret/virtsecretd.sysconf  |   3 -
 src/storage/meson.build |   5 --
 src/storage/virtstoraged.service.in |   1 +
 src/storage/virtstoraged.sysconf|   3 -
 src/vbox/meson.build|   5 --
 src/vbox/virtvboxd.service.in   |   1 +
 src/vbox/virtvboxd.sysconf  |   3 -
 src/vz/meson.build  |   5 --
 src/vz/virtvzd.service.in   |   1 +
 src/vz/virtvzd.sysconf  |   3 -
 tools/libvirt-guests.sh.in  |  40 ++
 tools/libvirt-guests.sysconf|  50 
 tools/meson.build   |   6 --
 54 files changed, 166 insertions(+), 260 deletions(-)
 delete mode 100644 src/ch/virtchd.sysconf
 delete mode 100644 src/interface/virtinterfaced.sysconf
 delete mode 100644 src/libxl/virtxend.sysconf
 delete mode 100644 src/locking/virtlockd.sysconf
 delete mode 100644 src/logging/virtlogd.sysconf
 delete mode 100644 src/lxc/virtlxcd.sysconf
 delete mode 100644 src/network/virtnetworkd.sysconf
 delete mode 100644 src/node_device/virtnodedevd.sysconf
 delete mode 100644 src/nwfilter/virtnwfilterd.sysconf
 delete mode 100644 src/qemu/virtqemud.sysconf
 delete mode 100644 src/remote/libvirtd.sysconf
 delete mode 100644 src/remote/virtproxyd.sysconf
 delete mode 100644 src/secret/virtsecretd.sysconf
 delete mode 100644 src/storage/virtstoraged.sysconf
 delete mode 100644 src/vbox/virtvboxd.sysconf
 delete mode 100644 src/vz/virtvzd.sysconf
 delete mode 100644 tools/libvirt-guests.sysconf

[libvirt PATCH] qemuxml2argvmock: drop virQEMUCapsGetKVMSupportsSecureGuest

[Pavel Hrdina]

It is actually not needed because in qemuxml2argvtest we preload
domaincapsmock as well.

Reported-by: Michal Privoznik 
Signed-off-by: Pavel Hrdina 
---
 tests/qemuxml2argvmock.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index d7f77eabf7..2265492f1e 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -41,7 +41,6 @@
 #include "virutil.h"
 #include "qemu/qemu_interface.h"
 #include "qemu/qemu_command.h"
-#include "qemu/qemu_capabilities.h"
 #include 
 #include 
 #include 
@@ -302,18 +301,3 @@ virIdentityEnsureSystemToken(void)
 {
 return g_strdup("3de80bcbf22d4833897f1638e01be9b2");
 }
-
-static bool (*real_virQEMUCapsGetKVMSupportsSecureGuest)(virQEMUCaps 
*qemuCaps);
-
-bool
-virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps)
-{
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) 
&&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_S390_PV_GUEST))
-return true;
-
-if (!real_virQEMUCapsGetKVMSupportsSecureGuest)
-VIR_MOCK_REAL_INIT(virQEMUCapsGetKVMSupportsSecureGuest);
-
-return real_virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps);
-}
-- 
2.31.1

Re: [libvirt PATCH] tests: fix compiling tests with minGW

[Pavel Hrdina]

On Thu, Jul 22, 2021 at 02:54:57PM +0200, Michal Prívozník wrote:
> On 7/22/21 12:24 PM, Pavel Hrdina wrote:
> > We need to mock virQEMUCapsGetKVMSupportsSecureGuest only if compiling
> > with QEMU otherwise compilation will fail with error:
> > 
> > /usr/lib/gcc/i686-w64-mingw32/11.1.1/../../../../i686-w64-mingw32/bin/ld: 
> > tests/libdomaincapsmock.dll.p/domaincapsmock.c.obj: in function 
> > `virQEMUCapsGetKVMSupportsSecureGuest':
> > /builds/libvirt/libvirt/build/../tests/domaincapsmock.c:40: undefined 
> > reference to `virQEMUCapsGet'
> > /usr/lib/gcc/i686-w64-mingw32/11.1.1/../../../../i686-w64-mingw32/bin/ld: 
> > /builds/libvirt/libvirt/build/../tests/domaincapsmock.c:41: undefined 
> > reference to `virQEMUCapsGet'
> > 
> > Fixes: 248a30c0c0ec2610e8fa2bbbc98da6d06978ad2e
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  tests/domaincapsmock.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> > index 7b02c0e890..0a6c541f77 100644
> > --- a/tests/domaincapsmock.c
> > +++ b/tests/domaincapsmock.c
> > @@ -17,8 +17,11 @@
> >  #include 
> >  
> >  #include "virhostcpu.h"
> > -#include "virmock.h"
> > -#include "qemu/qemu_capabilities.h"
> > +
> > +#if WITH_QEMU
> > +# include "virmock.h"
> > +# include "qemu/qemu_capabilities.h"
> > +#endif
> >  
> >  int
> >  virHostCPUGetKVMMaxVCPUs(void)
> > @@ -32,6 +35,7 @@ virHostCPUGetMicrocodeVersion(virArch hostArch 
> > G_GNUC_UNUSED)
> >  return 0;
> >  }
> >  
> > +#if WITH_QEMU
> >  static bool (*real_virQEMUCapsGetKVMSupportsSecureGuest)(virQEMUCaps 
> > *qemuCaps);
> >  
> >  bool
> > @@ -46,3 +50,4 @@ virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps 
> > *qemuCaps)
> >  
> >  return real_virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps);
> >  }
> > +#endif
> > 
> 
> Reviewed-by: Michal Privoznik 
> 
> However, this same code you're wrapping is in qemuxml2argvmock and I
> don't see much reason for that since qemuxml2argtest loads
> domaincapsmock too.

Didn't know that from top of my head and to be honest did not even think
about checking something like that :). Thanks for the info, I'll post
a patch removing the code from qemuxml2argvmock.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH] tests: fix compiling tests with minGW

[Michal Prívozník]

On 7/22/21 12:24 PM, Pavel Hrdina wrote:
> We need to mock virQEMUCapsGetKVMSupportsSecureGuest only if compiling
> with QEMU otherwise compilation will fail with error:
> 
> /usr/lib/gcc/i686-w64-mingw32/11.1.1/../../../../i686-w64-mingw32/bin/ld: 
> tests/libdomaincapsmock.dll.p/domaincapsmock.c.obj: in function 
> `virQEMUCapsGetKVMSupportsSecureGuest':
> /builds/libvirt/libvirt/build/../tests/domaincapsmock.c:40: undefined 
> reference to `virQEMUCapsGet'
> /usr/lib/gcc/i686-w64-mingw32/11.1.1/../../../../i686-w64-mingw32/bin/ld: 
> /builds/libvirt/libvirt/build/../tests/domaincapsmock.c:41: undefined 
> reference to `virQEMUCapsGet'
> 
> Fixes: 248a30c0c0ec2610e8fa2bbbc98da6d06978ad2e
> Signed-off-by: Pavel Hrdina 
> ---
>  tests/domaincapsmock.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> index 7b02c0e890..0a6c541f77 100644
> --- a/tests/domaincapsmock.c
> +++ b/tests/domaincapsmock.c
> @@ -17,8 +17,11 @@
>  #include 
>  
>  #include "virhostcpu.h"
> -#include "virmock.h"
> -#include "qemu/qemu_capabilities.h"
> +
> +#if WITH_QEMU
> +# include "virmock.h"
> +# include "qemu/qemu_capabilities.h"
> +#endif
>  
>  int
>  virHostCPUGetKVMMaxVCPUs(void)
> @@ -32,6 +35,7 @@ virHostCPUGetMicrocodeVersion(virArch hostArch 
> G_GNUC_UNUSED)
>  return 0;
>  }
>  
> +#if WITH_QEMU
>  static bool (*real_virQEMUCapsGetKVMSupportsSecureGuest)(virQEMUCaps 
> *qemuCaps);
>  
>  bool
> @@ -46,3 +50,4 @@ virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps)
>  
>  return real_virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps);
>  }
> +#endif
> 

Reviewed-by: Michal Privoznik 

However, this same code you're wrapping is in qemuxml2argvmock and I
don't see much reason for that since qemuxml2argtest loads
domaincapsmock too.

Michal

Re: [libvirt PATCH 07/10] virNWFilterParseParamAttributes: Remove superfluous `goto`s

[Jano Tomko]

On a %A in %Y, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake 
> ---
>  src/conf/nwfilter_params.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
> index 63ab7e7150..35ea0256c3 100644
> --- a/src/conf/nwfilter_params.c
> +++ b/src/conf/nwfilter_params.c
> @@ -710,28 +710,25 @@ virNWFilterParseParamAttributes(xmlNodePtr cur)
>  
>  if (nam == NULL || !isValidVarName(nam) ||
>  val == NULL || !isValidVarValue(val)) {
> -goto skip_entry;
> +cur = xmlNextElementSibling(cur);

You can also use a for loop if you don't want to duplicate this assignment.

Jano

> +continue;
>  }
>  

Re: [libvirt PATCH 00/10] virHashNew refactorings - part V

[Jano Tomko]

On a %A in %Y, Tim Wiederhake wrote:
> "virHashNew" cannot return NULL, yet we check for NULL in various places.
> 
> See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html.
> 
> Tim Wiederhake (10):
>   conf: Add AUTOPTR_CLEANUP_FUNC for virNWFilterVarValue
>   virNWFilterParseParamAttributes: `virHashNew` cannot return NULL
>   virNWFilterParseParamAttributes: Iterate over "element" children
>   virNWFilterParseParamAttributes: Remove tautological `if`
>   virNWFilterParseParamAttributes: Use automatic memory management
>   virNWFilterParseParamAttributes: Simplify loop body
>   virNWFilterParseParamAttributes: Remove superfluous `goto`s
>   virNWFilterDoInstantiate: `virHashNew` cannot return NULL
>   virNWFilterIPAddrMapInit: `virHashNew` cannot return NULL
>   virNWFilterLearnInit: `virHashNew` cannot return NULL
> 
>  src/conf/nwfilter_ipaddrmap.c  |  3 --
>  src/conf/nwfilter_params.c | 70 +-
>  src/conf/nwfilter_params.h |  2 +
>  src/nwfilter/nwfilter_gentech_driver.c |  5 --
>  src/nwfilter/nwfilter_learnipaddr.c|  8 ---
>  5 files changed, 26 insertions(+), 62 deletions(-)
> 

Reviewed-by: Ján Tomko 

Jano

Re: [PATCH v1] remove sysconfig files

[Olaf Hering]

Am Wed, 21 Jul 2021 12:36:40 +0100
schrieb Daniel P. Berrangé :

> If we remove the sysconfig files, we're not expecting users to
> modify the .service files. Instead they will be using the systemd
> overrides in /etc/systemd/system/libvirtd.service.d/ to
> customize.


It seems docs/daemons.rst already states that .service or .socket files can be 
tweaked, but it does not say how this needs to be done.

Looking at systemd.socket(8) and ListenStream=: "These options may be specified 
more than once, ..." indicates that one may not be able to tweak just one knob 
with an override file. Instead the full unit must be provided in /etc/systemd.

In the context of the proposed patch, a specific "Environment=" line will 
hopefully be override the system provided line reliably.

I will add an example to this documentation file. The existing wording 
regarding sysconfig files can stay as it is.


Olaf


pgpeYHhoyH5I7.pgp
Description: Digitale Signatur von OpenPGP

Re: [PATCH v1] remove sysconfig files

[Olaf Hering]

Am Wed, 21 Jul 2021 06:00:12 -0700
schrieb Andrea Bolognani :

> On Wed, Jul 21, 2021 at 01:23:56PM +0200, Olaf Hering wrote:
> > There is no point in restarting libvirtd in the middle of the transaction.
> > The spec file gives no ordering hints to rpm.  
> Note that the daemon restart always happens *after* the transaction,
> in the %posttrans part.

I was wrong, %systemd_post may not do a 'try-restart', but that certainly 
depends on what this macro actually does.
At least the macros I have will just do 'daemon-reload', they do nothing with 
the listed units.

It is not clear if 'try-restart' reloads the EnvironmentFile=.
If it is indeed reloaded, a modified file will not exist at this point because 
it was just renamed to .rpmsave.

In other words: the current '%post daemon' needs no modification.

> It looks like the only documentation for the various options that can
> be used to tweak the service's behavior was the one found in the
> sysconfig file. Perhaps a man page should be introduced?

Yeah, someone familiar with libvirt-guests.sh should provide one.
This patch just moves the existing comments elsewhere.
To mean it does not look like a user needs to tweak that script.

> But then libvirt-guests would end up being the only service that
> cannot be configured via a systemd override... I think we should
> strive for consistency here.

It certainly can be configured.
The environment can be constructed either with a .service.d/ file or with the 
(optional) sysconfig file.

Olaf


pgprGBwqURsBe.pgp
Description: Digitale Signatur von OpenPGP

Re: [PATCH] storage_driver: Unlock object on ACL fail in storagePoolLookupByTargetPath

[Michal Prívozník]

On 7/21/21 11:27 AM, Peter Krempa wrote:
> 'virStoragePoolObjListSearch' returns a locked and refed object, thus we
> must release it on ACL permission failure.
> 
> Fixes: 7aa0e8c0cb8
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
> Signed-off-by: Peter Krempa 
> ---
> Technically a security issue since it DoS-es the objects a user doesn't
> have access to for users which do have access.
> 
> Posting to standard devel list since the bugzilla above is public.
> 
>  src/storage/storage_driver.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Michal Privoznik 

Michal

[libvirt PATCH] tests: fix compiling tests with minGW

[Pavel Hrdina]

We need to mock virQEMUCapsGetKVMSupportsSecureGuest only if compiling
with QEMU otherwise compilation will fail with error:

/usr/lib/gcc/i686-w64-mingw32/11.1.1/../../../../i686-w64-mingw32/bin/ld: 
tests/libdomaincapsmock.dll.p/domaincapsmock.c.obj: in function 
`virQEMUCapsGetKVMSupportsSecureGuest':
/builds/libvirt/libvirt/build/../tests/domaincapsmock.c:40: undefined reference 
to `virQEMUCapsGet'
/usr/lib/gcc/i686-w64-mingw32/11.1.1/../../../../i686-w64-mingw32/bin/ld: 
/builds/libvirt/libvirt/build/../tests/domaincapsmock.c:41: undefined reference 
to `virQEMUCapsGet'

Fixes: 248a30c0c0ec2610e8fa2bbbc98da6d06978ad2e
Signed-off-by: Pavel Hrdina 
---
 tests/domaincapsmock.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
index 7b02c0e890..0a6c541f77 100644
--- a/tests/domaincapsmock.c
+++ b/tests/domaincapsmock.c
@@ -17,8 +17,11 @@
 #include 
 
 #include "virhostcpu.h"
-#include "virmock.h"
-#include "qemu/qemu_capabilities.h"
+
+#if WITH_QEMU
+# include "virmock.h"
+# include "qemu/qemu_capabilities.h"
+#endif
 
 int
 virHostCPUGetKVMMaxVCPUs(void)
@@ -32,6 +35,7 @@ virHostCPUGetMicrocodeVersion(virArch hostArch G_GNUC_UNUSED)
 return 0;
 }
 
+#if WITH_QEMU
 static bool (*real_virQEMUCapsGetKVMSupportsSecureGuest)(virQEMUCaps 
*qemuCaps);
 
 bool
@@ -46,3 +50,4 @@ virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps)
 
 return real_virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps);
 }
+#endif
-- 
2.31.1

[PATCH] storage: create logical volume with --yes option

[Kristina Hanicova]

If lvcreate found an existing signature when trying to create a
new logical volume (E.g. left after some deleted volume), the
action failed due to inability to answer interactive question to
wiping it (lvcreate assumed 'no' was the answer). With added
option --yes to the command line, the answer to any interactive
question is assumed to be yes. Therefore, lvcreate wipes the
signature and the new volume is created successfully.

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

Signed-off-by: Kristina Hanicova 
---
 src/storage/storage_backend_logical.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index ed8e47d880..02ede74aeb 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -863,6 +863,8 @@ virStorageBackendLogicalLVCreate(virStorageVolDef *vol,
 else
 virCommandAddArg(cmd, def->source.name);
 
+virCommandAddArg(cmd, "--yes");
+
 return virCommandRun(cmd, NULL);
 }
 
-- 
2.31.1

Re: [PATCH] storage_driver: Unlock object on ACL fail in storagePoolLookupByTargetPath

[Mauro Matteo Cascella]

Hi Peter,

I'm going to allocate a new (low impact) CVE for this bug.

Thanks.

On Wed, Jul 21, 2021 at 11:34 AM Peter Krempa  wrote:
>
> Adding libvirt-security since I forgot when sending the patch.
>
> On Wed, Jul 21, 2021 at 11:27:41 +0200, Peter Krempa wrote:
> > 'virStoragePoolObjListSearch' returns a locked and refed object, thus we
> > must release it on ACL permission failure.
> >
> > Fixes: 7aa0e8c0cb8
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
> > Signed-off-by: Peter Krempa 
> > ---
> > Technically a security issue since it DoS-es the objects a user doesn't
> > have access to for users which do have access.
> >
> > Posting to standard devel list since the bugzilla above is public.
> >
> >  src/storage/storage_driver.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> > index c2ff4b8d06..6aa10d89f6 100644
> > --- a/src/storage/storage_driver.c
> > +++ b/src/storage/storage_driver.c
> > @@ -1738,8 +1738,10 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
> > 
> > storagePoolLookupByTargetPathCallback,
> > cleanpath))) {
> >  def = virStoragePoolObjGetDef(obj);
> > -if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0)
> > +if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) {
> > +virStoragePoolObjEndAPI(&obj);
> >  return NULL;
> > +}
> >
> >  pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
> >  virStoragePoolObjEndAPI(&obj);
> > --
> > 2.31.1
> >
>


-- 
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0

Re: [PATCH] conf: remove unnecessary restore in virDomainSEVDefParseXML

[Pavel Hrdina]

On Thu, Jul 22, 2021 at 11:07:42AM +0200, Boris Fiuczynski wrote:
> Since commit d399a728f4 placed the restore in the right scope the
> restore can get removed in virDomainSEVDefParseXML.
> 
> Signed-off-by: Boris Fiuczynski 
> ---
>  src/conf/domain_conf.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

[PATCH] conf: remove unnecessary restore in virDomainSEVDefParseXML

[Boris Fiuczynski]

Since commit d399a728f4 placed the restore in the right scope the
restore can get removed in virDomainSEVDefParseXML.

Signed-off-by: Boris Fiuczynski 
---
 src/conf/domain_conf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 13b7741eb8..ac3f5ff4a3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14727,7 +14727,6 @@ static int
 virDomainSEVDefParseXML(virDomainSEVDef *def,
 xmlXPathContextPtr ctxt)
 {
-VIR_XPATH_NODE_AUTORESTORE(ctxt)
 unsigned long policy;
 int rc;
 
-- 
2.31.1

Re: [libvirt PATCH v2 2/2] ci: Halt on sanitizer errors

[Daniel P . Berrangé]

On Thu, Jul 22, 2021 at 11:00:18AM +0200, Tim Wiederhake wrote:
> The undefined behaviour sanitizer (UBSAN) defaults to merely printing an
> error message if it detects undefined behaviour. These error messages often
> end up in captured output and do not fail the tests, effectively hiding
> the warning. Make the test cases fail to make the issues visible.
> 
> Signed-off-by: Tim Wiederhake 
> ---
>  .gitlab-ci.yml | 2 ++
>  1 file changed, 2 insertions(+)

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 :|

Re: [libvirt PATCH v2 1/2] virFileReadLimFD: Cast maxlen to size_t before adding

[Daniel P . Berrangé]

On Thu, Jul 22, 2021 at 11:00:17AM +0200, Tim Wiederhake wrote:
> If the function is called with maxlen equal to `INT_MAX`, adding
> one will trigger a signed integer overflow.
> 
> Signed-off-by: Tim Wiederhake 
> ---
>  src/util/virfile.c | 2 +-
>  1 file changed, 1 insertion(+), 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 :|

[libvirt PATCH v2 2/2] ci: Halt on sanitizer errors

[Tim Wiederhake]

The undefined behaviour sanitizer (UBSAN) defaults to merely printing an
error message if it detects undefined behaviour. These error messages often
end up in captured output and do not fail the tests, effectively hiding
the warning. Make the test cases fail to make the issues visible.

Signed-off-by: Tim Wiederhake 
---
 .gitlab-ci.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3cb6ff5e6b..4757139fa9 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -89,6 +89,8 @@ stages:
 - meson build --werror -Ddocs=disabled -Db_lundef=false 
-Db_sanitize="$SANITIZER"
 - ninja -C build;
 - ninja -C build test;
+  variables:
+UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
 
 # Jobs that we delegate to Cirrus CI because they require an operating
 # system other than Linux. These jobs will only run if the required
-- 
2.31.1

[libvirt PATCH v2 0/2] Fail tests on UBSAN findings

[Tim Wiederhake]

V1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00592.html

Changes since V1:
Simplified series by using the approach mentioned in
https://listman.redhat.com/archives/libvir-list/2021-June/msg00358.html

Tim Wiederhake (2):
  virFileReadLimFD: Cast maxlen to size_t before adding
  ci: Halt on sanitizer errors

 .gitlab-ci.yml | 2 ++
 src/util/virfile.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.31.1

[libvirt PATCH v2 1/2] virFileReadLimFD: Cast maxlen to size_t before adding

[Tim Wiederhake]

If the function is called with maxlen equal to `INT_MAX`, adding
one will trigger a signed integer overflow.

Signed-off-by: Tim Wiederhake 
---
 src/util/virfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 723e1ca6e5..ad491251a2 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1422,7 +1422,7 @@ virFileReadLimFD(int fd, int maxlen, char **buf)
 errno = EINVAL;
 return -1;
 }
-s = saferead_lim(fd, maxlen+1, &len);
+s = saferead_lim(fd, (size_t) maxlen + 1, &len);
 if (s == NULL)
 return -1;
 if (len > maxlen || (int)len != len) {
-- 
2.31.1

Re: [libvirt PATCH 0/4] RFC: tests: introduce lavocado

[Daniel P . Berrangé]

On Wed, Jul 21, 2021 at 04:22:19PM -0300, Beraldo Leal wrote:
> On Wed, Jul 21, 2021 at 06:50:03PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 01, 2021 at 06:09:47PM -0300, Beraldo Leal wrote:
> > > On Thu, Jul 01, 2021 at 07:04:32PM +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Jun 30, 2021 at 01:36:30PM -0300, Beraldo Leal wrote:
> > > > > I'm adding more information with some details inside the README file.
> > > > 
> > > > Overall, I'm more enthusiastic about writing tests in Python
> > > > than Perl, for the long term, but would also potentially like
> > > > to write tests in Go too.
> > > > 
> > > > I'm wondering if we can't bridge the divide between what we
> > > > have already in libvirt-tck, and what you're bringing to the
> > > > table with avocado here. While we've not done much development
> > > > with the TCK recently, there are some very valuable tests
> > > > there, especially related to firewall support and I don't
> > > > fancy rewriting them.
> > > > 
> > > > Thus my suggestion is that we:
> > > > 
> > > >   - Put this avocado code into the libvirt-tck repository,
> > > > with focus on the supporting infra for making it easy to
> > > > write Python tests
> > > > 
> > > >   - Declare that all tests need a way to emit TAP format,
> > > > no matter what language they're written in. This could
> > > > either be the test directly emitting TAP, or it could
> > > > be via use of a plugin. For example 'tappy' can make
> > > > existing Python tests emit TAP, with no modifications
> > > > to the tests themselves.
> > > > 
> > > >   https://tappy.readthedocs.io/en/latest/consumers.html
> > > > 
> > > > IOW, you can still invoke the python tests using the
> > > > standard Python test runner, and still invoke the perl
> > > > tests using the stnadard Perl test runner if desired.
> > > 
> > > This is supported already:
> > > 
> > > $ avocado run --tap - --test-runner='nrunner' tests/domain/transient.py
> > > 1..3
> > > ok 1 tests/domain/transient.py:TransientDomain.test_autostart
> > > ok 2 tests/domain/transient.py:TransientDomain.test_lifecycle
> > > ok 3 
> > > tests/domain/transient.py:TransientDomain.test_convert_transient_to_persistent
> > 
> > This is nice, showing fine grained TAP output lines for each
> > individual test within the test program
> > 
> > 
> > I tried using the hints file that Cleber pointed to make avocado
> > *consume* TAP format for the Perl/Shell scripts:
> > 
> > $ cd libvirt-tck
> > $ cat .avocado.hint
> > [kinds]
> > tap = scripts/*/*.t
> > 
> > [tap]
> > uri = $testpath
> > 
> > 
> > And I can indeed invoke the scripts:
> > 
> > $ avocado run  ./scripts/domain/05*.t
> > JOB ID : b5d596d909dc8024d986957c909fc8fb6b31e2dd
> > JOB LOG: 
> > /home/berrange/avocado/job-results/job-2021-07-21T18.45-b5d596d/job.log
> >  (1/2) ./scripts/domain/050-transient-lifecycle.t: PASS (0.70 s)
> >  (2/2) ./scripts/domain/051-transient-autostart.t: PASS (0.76 s)
> > RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> > CANCEL 0
> > JOB HTML   : 
> > /home/berrange/avocado/job-results/job-2021-07-21T18.45-b5d596d/results.html
> > JOB TIME   : 1.90 s
> > 
> > which is good.
> > 
> > And also I can ask it to produce tap output too:
> > 
> > $ avocado run --tap - ./scripts/domain/05*.t
> > 1..2
> > ok 1 ./scripts/domain/050-transient-lifecycle.t
> > ok 2 ./scripts/domain/051-transient-autostart.t
> > 
> > 
> > But this output isn't entirely what I was after. This is just summarizing
> > the results of each test program.
> > 
> > I can't find a way to make it show the fine grained tap output for the
> > individual tests, like it does for the python program
> 
> Actually, the first Python TAP output example is showing a coarse TAP
> result. I have combined three transient tests into one single file but
> splitting them into three different methods, so we could execute each one
> individually. i.e: tests/domain/transient.py:TransientDomain.test_autostart.
> 
> So, I did some reorg when migrating to Python test.
> 
> In order to archive the same with Perl, we could do the same there,
> because the way individual tests are written there, doesn't allow for
> individual execution. 
> 
> Yes, we could do some tricks, to parse and combine outputs and list as
> it was a more fine graned, but afaict, we could not individually execute
> those. This is part of Avocado test definition where in order to be
> called a test, we need to be able to execute those individually as well.

Ok, I'm not so fussed about whether avocado can ultimately preserve the
fine grained TAP output. Mostly I'm looking to understand how you should
debug failures when they go wrong, becuase the default output I see from
avocado is very terse giving no indiciation of the the failure - merely
that there has been a failure.

In an interactive environment you can just re-rnu the individual failed
test directly. In an automated CI environment you need the t

Re: [libvirt PATCH 0/4] RFC: tests: introduce lavocado

[Daniel P . Berrangé]

On Wed, Jul 21, 2021 at 06:01:34PM -0400, Cleber Rosa wrote:
> 
> Daniel P. Berrangé writes:
> 
> > On Thu, Jul 01, 2021 at 06:09:47PM -0300, Beraldo Leal wrote:
> >> 
> >> This is supported already:
> >> 
> >> $ avocado run --tap - --test-runner='nrunner' tests/domain/transient.py
> >> 1..3
> >> ok 1 tests/domain/transient.py:TransientDomain.test_autostart
> >> ok 2 tests/domain/transient.py:TransientDomain.test_lifecycle
> >> ok 3 
> >> tests/domain/transient.py:TransientDomain.test_convert_transient_to_persistent
> >
> > This is nice, showing fine grained TAP output lines for each
> > individual test within the test program
> >
> >
> > I tried using the hints file that Cleber pointed to make avocado
> > *consume* TAP format for the Perl/Shell scripts:
> >
> > $ cd libvirt-tck
> > $ cat .avocado.hint
> > [kinds]
> > tap = scripts/*/*.t
> >
> > [tap]
> > uri = $testpath
> >
> >
> > And I can indeed invoke the scripts:
> >
> > $ avocado run  ./scripts/domain/05*.t
> > JOB ID : b5d596d909dc8024d986957c909fc8fb6b31e2dd
> > JOB LOG: 
> > /home/berrange/avocado/job-results/job-2021-07-21T18.45-b5d596d/job.log
> >  (1/2) ./scripts/domain/050-transient-lifecycle.t: PASS (0.70 s)
> >  (2/2) ./scripts/domain/051-transient-autostart.t: PASS (0.76 s)
> > RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> > CANCEL 0
> > JOB HTML   : 
> > /home/berrange/avocado/job-results/job-2021-07-21T18.45-b5d596d/results.html
> > JOB TIME   : 1.90 s
> >
> > which is good.
> >
> > And also I can ask it to produce tap output too:
> >
> > $ avocado run --tap - ./scripts/domain/05*.t
> > 1..2
> > ok 1 ./scripts/domain/050-transient-lifecycle.t
> > ok 2 ./scripts/domain/051-transient-autostart.t
> >
> >
> > But this output isn't entirely what I was after. This is just summarizing
> > the results of each test program.
> >
> 
> Hi Daniel,
> 
> From the user experience perspective, I completely understand your
> point.  I would side with this expectation and output any day if acting
> purely as a user.
> 
> But, as a framework, Avocado came up with some well defined concepts,
> and one of them is what a test is.  To be able to "replay only the
> failed tests of the latest job" or to "run a test completely isolated in
> a container or VM", there needs to be an specific and indistinguishable
> entry point for anything that is considered a test.

Ok, I see what you mean.

Regardless of what its written in, it is common that a test will
print arbitrary stuff to stdout or stderr if things go very badly
wrong, possibly including stack traces, etc.

What is the expected approach for debugging failed tests such that
you can see the full stdout/stderr ?  Are you supposed to just
directly invoke the individual test program without using the
avocado harness to see full output ?


> > I can't find a way to make it show the fine grained tap output for the
> > individual tests, like it does for the python program
> >
> > eg I'd like to be able to see something similar to:
> >
> > $ ./scripts/domain/050-transient-lifecycle.t
> > 1..2
> > # Creating a new transient domain
> > ok 1 - created transient domain object
> > # Destroying the transient domain
> > # Checking that transient domain has gone away
> > ok 2 - NO_DOMAIN error raised from missing domain
> >
> >
> > $ ./scripts/domain/051-transient-autostart.t
> > 1..4
> > # Creating a new transient domain
> > ok 1 - created transient domain object
> > ok 2 - autostart is disabled for transient VMs
> > ok 3 - Set autostart not supported on transient VMs
> > # Destroying the transient domain
> > # Checking that transient domain has gone away
> > ok 4 - NO_DOMAIN error raised from missing domain
> >
> 
> Results for tests that produce TAP can be seen in the granularity you
> list here, given some of the conditions like the one explained before.
> For instance, the GLib plugin[1] knows that it's possible to run a
> single test case on tests that use the g_test_*() APIs, by providing a
> "-p /test/case/name".
> 
> We could in theory reorganize the current Perl based tests, so that a
> test matches a function or some other type of code marker that can be
> used as an entry point.  Given that the overall result will always be
> valid and the log files will always contain the more granular
> information, I have mixed feelings about the cost/benefit of doing that,
> but you are surely a much better judge for that.

Yeah, that concept is alien to the Perl tests and not worth the effort
to try to change that - you'd be better off just rewriting from scratch
at that point. The perl re-execution granularity is purely at an
individual script level - the implication is that while in python you
might have a single test_foo.py with many tests inside, in Perl you'd
probably have many .t files.  

> > None the less this does seem like we're pretty close to being able
> > to do something useful in integration
> 
> PS: The GLib plugin was deprecated because we observed that most users
>

Re: [PULL 36/40] vl: switch -M parsing to keyval

[Peter Krempa]

CC libvir-list

On Tue, Jul 06, 2021 at 12:01:37 +0200, Paolo Bonzini wrote:
> Switch from QemuOpts to keyval.  This enables the introduction
> of non-scalar machine properties, and JSON syntax in the future.
> 
> For JSON syntax to be supported right now, we would have to
> consider what would happen if string-based dictionaries (produced by
> -M key=val) were to be merged with strongly-typed dictionaries
> (produced by -M {'key': 123}).
> 
> The simplest way out is to never enter the situation, and only allow one
> -M option when JSON syntax is in use.  However, we want options such as
> -smp to become syntactic sugar for -M, and this is a problem; as soon
> as -smp becomes a shortcut for -M, QEMU would forbid using -M '{}'
> together with -smp.  Therefore, allowing JSON syntax right now for -M
> would be a forward-compatibility nightmare and it would be impossible
> anyway to introduce -M incrementally in tools.
> 
> Instead, support for JSON syntax is delayed until after the main
> options are converted to QOM compound properties.  These include -boot,
> -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg.  Once JSON
> syntax is introduced, these options will _also_ be forbidden together
> with -M '{...}'.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  softmmu/vl.c | 315 ---
>  1 file changed, 146 insertions(+), 169 deletions(-)

This patch breaks detection of certain machine options features in
libvirt which were being detected from 'query-command-line-options'.

I presume the change simply removed this from the output of
query-command-line-options due to the historical cruft how the
aforementioned command works.

Unfortunately I didn't find any suitable replacement from what we are
querying.

The entries which we now lack detection are:

{ "machine", "mem-merge", QEMU_CAPS_MEM_MERGE },
{ "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
{ "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP },
{ "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP },
{ "machine", "loadparm", QEMU_CAPS_LOADPARM },

Note that the oldest supported qemu version in libvirt is 2.11 at this
point and all the above flags are included in 2.11 and later.

This means we can theoretically remove the detection and always assume
the flags in case when they are unlikely to be removed in the future.

Alternatively if you can suggest an option how to detect this it will be
welcome.

Thanks

Peter

Re: [libvirt PATCH 4/4] ci: Halt on sanitizer errors

[Tim Wiederhake]

On Wed, 2021-07-21 at 06:56 -0700, Andrea Bolognani wrote:
> On Wed, Jul 21, 2021 at 03:08:02PM +0200, Peter Krempa wrote:
> > On Wed, Jul 21, 2021 at 14:46:43 +0200, Tim Wiederhake wrote:
> > > +++ b/.gitlab-ci.yml
> > > @@ -89,6 +89,8 @@ stages:
> > >  - meson build --werror -Ddocs=disabled -Db_lundef=false -
> > > Db_sanitize="$SANITIZER"
> > >  - ninja -C build;
> > >  - ninja -C build test;
> > > +  variables:
> > > +    UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1
> > 
> > Is this being propagated as an env variable? In many cases in the
> > gitlab-ci there are entries doing 'export ENV=VAL' for some reason.
> 
> Setting environment variables as part of the script rather than using
> the native GitLab CI keyword is necessary to be able to use the same
> environment for multiple jobs: using something like
> 
>   .template:
>     variables:
>   FOO: foo
> 
>   job:
>     extends: .template
>     variables:
>   BAR: bar
> 
> would result in only $BAR being set for job, which is not what we
> want. We always use 'variables' where possible.
> 

I am not sure that I understand you correctly:

.gitlab-ci.yml:
.template:
  variables:
MARKER_FOO: foo
  script: "env | grep MARKER"

job:
  extends: .template
  variables:
MARKER_BAR: bar

Both "MARKER_FOO" and "MARKER_BAR" are set:
https://gitlab.com/twiederh/libvirt/-/jobs/1443690227

Here is a pipeline for a branch with only patch #4, which fails as
expected:
https://gitlab.com/twiederh/libvirt/-/pipelines/340558056

Is it possible that that Gitlab used to exhibit the behavior that you
describe, but it was fixed in the meantime?

Regards,
Tim

[PATCH v4 2/3] util: Add virHostCPUGetHaltPollTime

[Yang Fei]

Add helper function virHostCPUGetHaltPollTime to obtain halt polling
time. If the kernel support halt polling time statistic, and mount
debugfs. This function will take effect on KVM VMs.

Signed-off-by: Yang Fei 
---
 src/libvirt_private.syms |  1 +
 src/util/virhostcpu.c| 39 +++
 src/util/virhostcpu.h|  4 
 3 files changed, 44 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5429a82a7c..531ba03cf7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2374,6 +2374,7 @@ virHookPresent;
 # util/virhostcpu.h
 virHostCPUGetAvailableCPUsBitmap;
 virHostCPUGetCount;
+virHostCPUGetHaltPollTime;
 virHostCPUGetInfo;
 virHostCPUGetKVMMaxVCPUs;
 virHostCPUGetMap;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index bf7fda23af..7f577c3e3e 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1535,3 +1535,42 @@ virHostCPUGetSignature(char **signature)
 }
 
 #endif /* __linux__ */
+
+int
+virHostCPUGetHaltPollTime(pid_t pid,
+  unsigned long long *haltPollSuccess,
+  unsigned long long *haltPollFail)
+{
+g_autofree char *pidToStr = NULL;
+g_autofree char *debugFsPath = NULL;
+g_autofree char *kvmPath = NULL;
+struct dirent *ent = NULL;
+g_autoptr(DIR) dir = NULL;
+bool found = false;
+
+if (!(debugFsPath = virFileFindMountPoint("debugfs")))
+return -1;
+
+kvmPath = g_strdup_printf("%s/%s", debugFsPath, "kvm");
+if (virDirOpenQuiet(&dir, kvmPath) != 1)
+return -1;
+
+pidToStr = g_strdup_printf("%lld%c", (long long)pid, '-');
+while (virDirRead(dir, &ent, NULL) > 0) {
+if (STRPREFIX(ent->d_name, pidToStr)) {
+found = true;
+break;
+}
+}
+
+if (!found)
+return -1;
+
+if (virFileReadValueUllongQuiet(haltPollSuccess, "%s/%s/%s", kvmPath,
+ent->d_name, "halt_poll_success_ns") < 0 ||
+virFileReadValueUllongQuiet(haltPollFail, "%s/%s/%s", kvmPath,
+ent->d_name, "halt_poll_fail_ns") < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index fc717250d9..d98385d53f 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -83,3 +83,7 @@ int virHostCPUGetMSR(unsigned long index,
 virHostCPUTscInfo *virHostCPUGetTscInfo(void);
 
 int virHostCPUGetSignature(char **signature);
+
+int virHostCPUGetHaltPollTime(pid_t pid,
+  unsigned long long *haltPollSuccess,
+  unsigned long long *haltPollFail);
-- 
2.23.0

[PATCH v4 0/3] domstats:add haltpolling time statistic interface

[Yang Fei]

This series add the ability to statistic the halt polling time when
VM execute HLT(arm is WFI).

v1:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00029.html
v2:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00339.html
v3:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00445.html

changes from v1:
- Move virGetCgroupValueRaw to utils.c and rename it virGetValueRaw. So
  that we can call it to obtain halt polling time.
- Helper function virGetCpuHaltPollTime and virGetDebugFsKvmValue are
  added in a separate patch
- Use STRPREFIX to match the path prefix.
- Fix the logic that domstats will break when platform is non-linux,
  debugfs isn't mounted and so on.

change from v2:
- Drop patch 1, use virFileReadValueUllong() to get halt polling data.
- Delete unnecessary error report in logs.
- Remove the qemuDomainGetStatsCpuHaltPollTime function conditionally
  compiled on linux.
- Document the new parameters in src/libvirt-domain.c.

change from v3:
- Add function virFileReadValueUllongQuiet without error report.
- Move virGetCpuHaltPollTime to src/util/virhostcpu.c and change the
  name to virHostCPUGetHaltPollTime.
- Replace the function which will report errors:
  virDirOpenIfExists -> virDirOpenQuiet
  virFileReadValueUllong -> virFileReadValueUllongQuiet

Yang Fei (3):
  util: Add virFileReadValueUllongQuiet
  util: Add virHostCPUGetHaltPollTime
  qemu: Introduce qemuDomainGetStatsCpuHaltPollTime

 docs/manpages/virsh.rst  |  4 
 src/libvirt-domain.c |  7 +++
 src/libvirt_private.syms |  2 ++
 src/qemu/qemu_driver.c   | 20 
 src/util/virfile.c   | 24 
 src/util/virfile.h   |  2 ++
 src/util/virhostcpu.c| 39 +++
 src/util/virhostcpu.h|  4 
 8 files changed, 102 insertions(+)

-- 
2.23.0

[PATCH v4 3/3] qemu: Introduce qemuDomainGetStatsCpuHaltPollTime

[Yang Fei]

This function add halt polling time interface in domstats. So that
we can use command 'virsh domstats VM' to get the data if system
support.

Signed-off-by: Yang Fei 
---
 docs/manpages/virsh.rst |  4 
 src/libvirt-domain.c|  7 +++
 src/qemu/qemu_driver.c  | 20 
 3 files changed, 31 insertions(+)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 87668f2b9a..20936994ce 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2260,6 +2260,10 @@ When selecting the *--state* group the following fields 
are returned:
 * ``cpu.time`` - total cpu time spent for this domain in nanoseconds
 * ``cpu.user`` - user cpu time spent in nanoseconds
 * ``cpu.system`` - system cpu time spent in nanoseconds
+* ``cpu.haltpoll.success.time`` - cpu halt polling success time spent in
+  nanoseconds
+* ``cpu.haltpoll.fail.time`` - cpu halt polling fail time spent in
+  nanoseconds
 * ``cpu.cache.monitor.count`` - the number of cache monitors for this
   domain
 * ``cpu.cache.monitor..name`` - the name of cache monitor 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 35c0df0ebc..4eb14d4176 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11625,6 +11625,13 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
  * "cpu.system" - system cpu time spent in nanoseconds as unsigned long
  *long.
+ * "cpu.haltpoll.success.time" - halt-polling cpu usage about the VCPU 
polled
+ *   until a virtual interrupt was delivered in
+ *   nanoseconds as unsigned long long.
+ * "cpu.haltpoll.fail.time" - halt-polling cpu usage about the VCPU had to 
schedule
+ *out (either because the maximum poll time 
was reached
+ *or it needed to yield the CPU) in 
nanoseconds as
+ *unsigned long long.
  * "cpu.cache.monitor.count" - the number of cache monitors for this domain
  * "cpu.cache.monitor..name" - the name of cache monitor 
  * "cpu.cache.monitor..vcpus" - vcpu list of cache monitor 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 521063d438..79308b7157 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17857,6 +17857,23 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom,
 return 0;
 }
 
+static int
+qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
+  virTypedParamList *params)
+{
+unsigned long long haltPollSuccess = 0;
+unsigned long long haltPollFail = 0;
+pid_t pid = dom->pid;
+
+if (virHostCPUGetHaltPollTime(pid, &haltPollSuccess, &haltPollFail) < 0)
+return 0;
+
+if (virTypedParamListAddULLong(params, haltPollSuccess, 
"cpu.haltpoll.success.time") < 0 ||
+virTypedParamListAddULLong(params, haltPollFail, 
"cpu.haltpoll.fail.time") < 0)
+return -1;
+
+return 0;
+}
 
 static int
 qemuDomainGetStatsCpu(virQEMUDriver *driver,
@@ -17870,6 +17887,9 @@ qemuDomainGetStatsCpu(virQEMUDriver *driver,
 if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0)
 return -1;
 
+if (qemuDomainGetStatsCpuHaltPollTime(dom, params) < 0)
+return -1;
+
 return 0;
 }
 
-- 
2.23.0

[PATCH v4 1/3] util: Add virFileReadValueUllongQuiet

[Yang Fei]

Use function virFileReadValueUllongQuiet to read unsigned long
long value without error report.

Signed-off-by: Yang Fei 
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 24 
 src/util/virfile.h   |  2 ++
 3 files changed, 27 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4bc2974e7f..5429a82a7c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2242,6 +2242,7 @@ virFileReadValueScaledInt;
 virFileReadValueString;
 virFileReadValueUint;
 virFileReadValueUllong;
+virFileReadValueUllongQuiet;
 virFileRelLinkPointsTo;
 virFileRemove;
 virFileRemoveLastComponent;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 723e1ca6e5..70737f4751 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4065,6 +4065,30 @@ virFileReadValueUllong(unsigned long long *value, const 
char *format, ...)
 return 0;
 }
 
+int
+virFileReadValueUllongQuiet(unsigned long long *value, const char *format, ...)
+{
+g_autofree char *str = NULL;
+g_autofree char *path = NULL;
+va_list ap;
+
+va_start(ap, format);
+path = g_strdup_vprintf(format, ap);
+va_end(ap);
+
+if (!virFileExists(path))
+return -2;
+
+if (virFileReadAllQuiet(path, VIR_INT64_STR_BUFLEN, &str) < 0)
+return -1;
+
+virStringTrimOptionalNewline(str);
+
+if (virStrToLong_ullp(str, NULL, 10, value) < 0)
+return -1;
+
+return 0;
+}
 
 /**
  * virFileReadValueScaledInt:
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 72368495bf..967c9a9b4f 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -339,6 +339,8 @@ int virFileReadValueUint(unsigned int *value, const char 
*format, ...)
  G_GNUC_PRINTF(2, 3);
 int virFileReadValueUllong(unsigned long long *value, const char *format, ...)
  G_GNUC_PRINTF(2, 3);
+int virFileReadValueUllongQuiet(unsigned long long *value, const char *format, 
...)
+ G_GNUC_PRINTF(2, 3);
 int virFileReadValueBitmap(virBitmap **value, const char *format, ...)
  G_GNUC_PRINTF(2, 3);
 int virFileReadValueScaledInt(unsigned long long *value, const char *format, 
...)
-- 
2.23.0

RE: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support

[Duan, Zhenzhong]

> -Original Message-
> From: Pavel Hrdina 
> Sent: Wednesday, July 21, 2021 10:23 PM
> To: Duan, Zhenzhong 
> Cc: libvir-list@redhat.com; pkre...@redhat.com; berra...@redhat.com;
> Yamahata, Isaku ; Tian, Jun J
> ; Qiang, Chenyi 
> Subject: Re: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support
> 
> On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
> > Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2
> comes.
> >
> > * What's TDX?
> > TDX stands for Trust Domain Extensions which isolates VMs from the
> > virtual-machine manager (VMM)/hypervisor and any other software on the
> > platform.
[...]
> > * Misc
> > Just let you know we have released v2 version of TDX qemu in [1], and
> > the API for libvirt is keeping stable. Using these patches we have
> > succesfully booted and tested a guest both with and without TDX enabled.
> 
> Overall looks good. It's missing documentation and the QEMU patches are
> missing documentation as well. I was looking into Intel specification but I
> failed to find the necessary info there as well.
> What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data
> is supposed to be stored there, what are the limitation and so on.
Oh, yes. Thanks for point out. We will add the doc both for qemu and libvirt.

> 
> What I could gather these are exposed in the VM and are used for
> measurement but that's it.
> 
> Another thing that I've missed in v1, QEMU patches are introducing new `-
> machine pic=no` option and for TDX PIC has to be disabled. The libvirt
> patches are putting it on the QEMU command line but it is not reflected in
> the VM XML, so I would say we need to introduce new hypervisor feature [1]:
> 
>   
> ...
> 
> ...
>   
> 
> [1] 
Will add this feature.

> 
> > * Diff to v1:
> > - give up using qmp cmd and check TDX directly on host for TDX capabilities.
> > - use launchsecurity framework to support TDX
> > - use . for general loader
> > - add auto firmware match feature for TDX
> >
> > A example TDVF fimware description file 70-edk2-x86_64-tdx.json:
> > {
> > "description": "UEFI firmware for x86_64, supporting Intel TDX",
> > "interface-types": [
> > "uefi"
> > ],
> > "mapping": {
> > "device": "generic",
> 
> I think using 'loader' as that's the actual device in QEMU used with this
> firmware will be better. The patches posted to QEMU doesn't extend
> `docs/interop/firmware.json` so this example may change once some specific
> format is accepted by QEMU community.
Will do.
> 
> You will most likely need to add the firmware descriptor to QEMU project as
> well (`pc-bios/descriptors/70-edk2-x86_64-tdx.json`). NOTE: The name
> should not use `edk2` if it's not edk2 based firmware.
I see, will do. Thanks very much for your suggestions.

Regards
Zhenzhong

Re: [PATCH] conf: Restore ctxt's node in right scope

[Pavel Hrdina]

On Thu, Jul 22, 2021 at 03:44:18PM +0800, Zhenzhong Duan wrote:
> We just found  is ignored in our xml. Further debug
> shows that ctxt's node pointer isn't restored in virDomainSecDefParseXML(),
> which leads to parsing of remaining elements failed.
> 
> Signed-off-by: Zhenzhong Duan 
> ---
>  src/conf/domain_conf.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

[libvirt PATCH 11/38] libxlLoggerNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/libxl/libxl_logger.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c
index 0807383d39..f7b5c8ee16 100644
--- a/src/libxl/libxl_logger.c
+++ b/src/libxl/libxl_logger.c
@@ -149,9 +149,7 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)
 break;
 }
 logger.logDir = logDir;
-
-if ((logger.files = virHashNew(libxlLoggerFileFree)) == NULL)
-return NULL;
+logger.files = virHashNew(libxlLoggerFileFree);
 
 path = g_strdup_printf("%s/libxl-driver.log", logDir);
 
-- 
2.31.1

[libvirt PATCH 28/38] virLockSpaceNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virlockspace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 9986d8df41..edf2ec907f 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -247,8 +247,7 @@ virLockSpace *virLockSpaceNew(const char *directory)
 
 lockspace->dir = g_strdup(directory);
 
-if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree)))
-goto error;
+lockspace->resources = virHashNew(virLockSpaceResourceDataFree);
 
 if (directory) {
 if (virFileExists(directory)) {
-- 
2.31.1

[libvirt PATCH 30/38] virNetDaemonNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/rpc/virnetdaemon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 1a17753f42..b6f3233f64 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -141,8 +141,7 @@ virNetDaemonNew(void)
 if (!(dmn = virObjectLockableNew(virNetDaemonClass)))
 return NULL;
 
-if (!(dmn->servers = virHashNew(virObjectFreeHashData)))
-goto error;
+dmn->servers = virHashNew(virObjectFreeHashData);
 
 #ifndef WIN32
 dmn->sigwrite = dmn->sigread = -1;
-- 
2.31.1

[libvirt PATCH 36/38] virSecuritySELinuxQEMUInitialize: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/security/security_selinux.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 0bff9b7bae..5f98d4d47a 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -758,8 +758,7 @@ virSecuritySELinuxQEMUInitialize(virSecurityManager *mgr)
 VIR_DEBUG("Loaded file context '%s', content context '%s'",
   data->file_context, data->content_context);
 
-if (!(data->mcs = virHashNew(NULL)))
-goto error;
+data->mcs = virHashNew(NULL);
 
 return 0;
 
-- 
2.31.1

[libvirt PATCH 32/38] virNetworkObjNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virnetworkobj.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 66f916c749..ea021892c7 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -118,8 +118,7 @@ virNetworkObjNew(void)
 virBitmapSetBitExpand(obj->classIdMap, 2) < 0)
 goto error;
 
-if (!(obj->ports = virHashNew(virNetworkObjPortFree)))
-goto error;
+obj->ports = virHashNew(virNetworkObjPortFree);
 
 virObjectLock(obj);
 
-- 
2.31.1

[libvirt PATCH 09/38] hypervCreateEmbeddedParam: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/hyperv/hyperv_wmi.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index b113283aeb..dc2c6471ab 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -275,7 +275,7 @@ hypervCreateEmbeddedParam(hypervWmiClassInfo *classInfo)
 {
 size_t i;
 size_t count;
-g_autoptr(GHashTable) table = NULL;
+g_autoptr(GHashTable) table = virHashNew(NULL);
 XmlSerializerInfo *typeinfo = NULL;
 
 typeinfo = classInfo->serializerInfo;
@@ -284,10 +284,6 @@ hypervCreateEmbeddedParam(hypervWmiClassInfo *classInfo)
 for (count = 0; typeinfo[count].name != NULL; count++)
 ;
 
-table = virHashNew(NULL);
-if (table == NULL)
-return NULL;
-
 for (i = 0; typeinfo[i].name != NULL; i++) {
 XmlSerializerInfo *item = &typeinfo[i];
 
-- 
2.31.1

[libvirt PATCH 37/38] virStoragePoolObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virstorageobj.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 24957d6012..6c8a06e8bc 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -401,12 +401,8 @@ virStoragePoolObjListNew(void)
 if (!(pools = virObjectRWLockableNew(virStoragePoolObjListClass)))
 return NULL;
 
-if (!(pools->objs = virHashNew(virObjectFreeHashData)) ||
-!(pools->objsName = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(pools);
-return NULL;
-}
-
+pools->objs = virHashNew(virObjectFreeHashData);
+pools->objsName = virHashNew(virObjectFreeHashData);
 return pools;
 }
 
-- 
2.31.1

[libvirt PATCH 38/38] virStorageVolObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virstorageobj.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 6c8a06e8bc..2b6932d7e5 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -170,12 +170,9 @@ virStorageVolObjListNew(void)
 if (!(vols = virObjectRWLockableNew(virStorageVolObjListClass)))
 return NULL;
 
-if (!(vols->objsKey = virHashNew(virObjectFreeHashData)) ||
-!(vols->objsName = virHashNew(virObjectFreeHashData)) ||
-!(vols->objsPath = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(vols);
-return NULL;
-}
+vols->objsKey = virHashNew(virObjectFreeHashData);
+vols->objsName = virHashNew(virObjectFreeHashData);
+vols->objsPath = virHashNew(virObjectFreeHashData);
 
 return vols;
 }
-- 
2.31.1

[libvirt PATCH 35/38] virSecuritySELinuxLXCInitialize: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/security/security_selinux.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 0c2cf1d1c7..0bff9b7bae 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -687,8 +687,7 @@ virSecuritySELinuxLXCInitialize(virSecurityManager *mgr)
 goto error;
 }
 
-if (!(data->mcs = virHashNew(NULL)))
-goto error;
+data->mcs = virHashNew(NULL);
 
 return 0;
 
-- 
2.31.1

[libvirt PATCH 34/38] virQEMUCapsProbeQMPHostCPU: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/qemu/qemu_capabilities.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6cea8c2eca..3804687080 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3045,8 +3045,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps,
 qemuMonitorCPUProperty *nmProp;
 size_t i;
 
-if (!(hash = virHashNew(NULL)))
-goto cleanup;
+hash = virHashNew(NULL);
 
 for (i = 0; i < modelInfo->nprops; i++) {
 prop = modelInfo->props + i;
-- 
2.31.1

[libvirt PATCH 33/38] virNodeDeviceObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virnodedeviceobj.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index b213592b56..e8f6792138 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -456,10 +456,7 @@ virNodeDeviceObjListNew(void)
 if (!(devs = virObjectRWLockableNew(virNodeDeviceObjListClass)))
 return NULL;
 
-if (!(devs->objs = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(devs);
-return NULL;
-}
+devs->objs = virHashNew(virObjectFreeHashData);
 
 return devs;
 }
-- 
2.31.1

[libvirt PATCH 29/38] virLockSpaceNewPostExecRestart: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virlockspace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index edf2ec907f..0c4beaf9fe 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -293,8 +293,7 @@ virLockSpace *virLockSpaceNewPostExecRestart(virJSONValue 
*object)
 return NULL;
 }
 
-if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree)))
-goto error;
+virHashNew(virLockSpaceResourceDataFree);
 
 if (virJSONValueObjectHasKey(object, "directory")) {
 const char *dir = virJSONValueObjectGetString(object, "directory");
-- 
2.31.1

[libvirt PATCH 31/38] virNetworkObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virnetworkobj.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 341d6b4292..66f916c749 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -350,10 +350,7 @@ virNetworkObjListNew(void)
 if (!(nets = virObjectRWLockableNew(virNetworkObjListClass)))
 return NULL;
 
-if (!(nets->objs = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(nets);
-return NULL;
-}
+nets->objs = virHashNew(virObjectFreeHashData);
 
 return nets;
 }
-- 
2.31.1

[libvirt PATCH 27/38] virLockDaemonNewPostExecRestart: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/locking/lock_daemon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index f5e5375e80..fa08acbc76 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -211,8 +211,7 @@ virLockDaemonNewPostExecRestart(virJSONValue *object, bool 
privileged)
 
 g_mutex_init(&lockd->lock);
 
-if (!(lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree)))
-goto error;
+lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree);
 
 if (!(child = virJSONValueObjectGet(object, "defaultLockspace"))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-- 
2.31.1

[libvirt PATCH 26/38] virLockDaemonNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/locking/lock_daemon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 05fc68dc54..f5e5375e80 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -153,8 +153,7 @@ virLockDaemonNew(virLockDaemonConfig *config, bool 
privileged)
 virObjectUnref(srv);
 srv = NULL;
 
-if (!(lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree)))
-goto error;
+lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree);
 
 if (!(lockd->defaultLockspace = virLockSpaceNew(NULL)))
 goto error;
-- 
2.31.1

[libvirt PATCH 24/38] virHashAtomicNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virhash.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/util/virhash.c b/src/util/virhash.c
index d2b591c0b7..a9996c9fc0 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -102,10 +102,7 @@ virHashAtomicNew(virHashDataFree dataFree)
 if (!(hash = virObjectLockableNew(virHashAtomicClass)))
 return NULL;
 
-if (!(hash->hash = virHashNew(dataFree))) {
-virObjectUnref(hash);
-return NULL;
-}
+hash->hash = virHashNew(dataFree);
 return hash;
 }
 
-- 
2.31.1

[libvirt PATCH 25/38] virInterfaceObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virinterfaceobj.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index c8fda4990f..a73208f1fc 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -142,10 +142,7 @@ virInterfaceObjListNew(void)
 if (!(interfaces = virObjectRWLockableNew(virInterfaceObjListClass)))
 return NULL;
 
-if (!(interfaces->objsName = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(interfaces);
-return NULL;
-}
+interfaces->objsName = virHashNew(virObjectFreeHashData);
 
 return interfaces;
 }
-- 
2.31.1

[libvirt PATCH 23/38] virFileCacheNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virfilecache.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index 2f55deefb6..64348dc1e6 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -242,8 +242,7 @@ virFileCacheNew(const char *dir,
 if (!(cache = virObjectNew(virFileCacheClass)))
 return NULL;
 
-if (!(cache->table = virHashNew(virObjectFreeHashData)))
-goto cleanup;
+cache->table = virHashNew(virObjectFreeHashData);
 
 cache->dir = g_strdup(dir);
 
@@ -252,10 +251,6 @@ virFileCacheNew(const char *dir,
 cache->handlers = *handlers;
 
 return cache;
-
- cleanup:
-virObjectUnref(cache);
-return NULL;
 }
 
 
-- 
2.31.1

[libvirt PATCH 21/38] virDomainMomentObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virdomainmomentobjlist.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/conf/virdomainmomentobjlist.c 
b/src/conf/virdomainmomentobjlist.c
index c0896685a9..17b9c16ae7 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -274,10 +274,6 @@ virDomainMomentObjListNew(void)
 
 moments = g_new0(virDomainMomentObjList, 1);
 moments->objs = virHashNew(virDomainMomentObjListDataFree);
-if (!moments->objs) {
-VIR_FREE(moments);
-return NULL;
-}
 return moments;
 }
 
-- 
2.31.1

[libvirt PATCH 22/38] virDomainObjListNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virdomainobjlist.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 5f9fd9aabc..43d09692a9 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -75,12 +75,8 @@ virDomainObjList *virDomainObjListNew(void)
 if (!(doms = virObjectRWLockableNew(virDomainObjListClass)))
 return NULL;
 
-if (!(doms->objs = virHashNew(virObjectFreeHashData)) ||
-!(doms->objsName = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(doms);
-return NULL;
-}
-
+doms->objs = virHashNew(virObjectFreeHashData);
+doms->objsName = virHashNew(virObjectFreeHashData);
 return doms;
 }
 
-- 
2.31.1

[libvirt PATCH 20/38] virDomainDefValidateAliases: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_validate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index df2ab47361..aab377fbbd 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1261,8 +1261,7 @@ virDomainDefValidateAliases(const virDomainDef *def,
 int ret = -1;
 
 /* We are not storing copies of aliases. Don't free them. */
-if (!(data.aliases = virHashNew(NULL)))
-goto cleanup;
+data.aliases = virHashNew(NULL);
 
 if (virDomainDeviceInfoIterateFlags((virDomainDef *) def,
 
virDomainDeviceDefValidateAliasesIterator,
-- 
2.31.1

[libvirt PATCH 19/38] virDomainDefBootOrderPostParse: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e05ea9ba88..6937863db7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5924,10 +5924,7 @@ virDomainDefCollectBootOrder(virDomainDef *def 
G_GNUC_UNUSED,
 static int
 virDomainDefBootOrderPostParse(virDomainDef *def)
 {
-g_autoptr(GHashTable) bootHash = NULL;
-
-if (!(bootHash = virHashNew(NULL)))
-return -1;
+g_autoptr(GHashTable) bootHash = virHashNew(NULL);
 
 if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder, 
bootHash) < 0)
 return -1;
-- 
2.31.1

[libvirt PATCH 18/38] virDomainCCWAddressSetCreate: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_addr.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index f011721beb..53b39923e8 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1388,8 +1388,7 @@ virDomainCCWAddressSetCreate(void)
 
 addrs = g_new0(virDomainCCWAddressSet, 1);
 
-if (!(addrs->defined = virHashNew(g_free)))
-goto error;
+addrs->defined = virHashNew(g_free);
 
 /* must use cssid = 0xfe (254) for virtio-ccw devices */
 addrs->next.cssid = 254;
@@ -1397,10 +1396,6 @@ virDomainCCWAddressSetCreate(void)
 addrs->next.devno = 0;
 addrs->next.assigned = 0;
 return addrs;
-
- error:
-virDomainCCWAddressSetFree(addrs);
-return NULL;
 }
 
 
-- 
2.31.1

[libvirt PATCH 17/38] virCloseCallbacksNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/hypervisor/virclosecallbacks.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/hypervisor/virclosecallbacks.c 
b/src/hypervisor/virclosecallbacks.c
index b9d4940b60..2578a71f03 100644
--- a/src/hypervisor/virclosecallbacks.c
+++ b/src/hypervisor/virclosecallbacks.c
@@ -69,10 +69,6 @@ virCloseCallbacksNew(void)
 return NULL;
 
 closeCallbacks->list = virHashNew(g_free);
-if (!closeCallbacks->list) {
-virObjectUnref(closeCallbacks);
-return NULL;
-}
 
 return closeCallbacks;
 }
-- 
2.31.1

[libvirt PATCH 16/38] virChrdevAlloc: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/virchrdev.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index 91f2b5a233..5d6de68427 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -265,13 +265,9 @@ virChrdevs *virChrdevAlloc(void)
 
 /* there will hardly be any devices most of the time, the hash
  * does not have to be huge */
-if (!(devs->hash = virHashNew(virChrdevHashEntryFree)))
-goto error;
+devs->hash = virHashNew(virChrdevHashEntryFree);
 
 return devs;
- error:
-virChrdevFree(devs);
-return NULL;
 }
 
 /**
-- 
2.31.1

[libvirt PATCH 14/38] qemuDomainObjPrivateAlloc: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/qemu/qemu_domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ac1d8ef151..b919da6eab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1788,8 +1788,7 @@ qemuDomainObjPrivateAlloc(void *opaque)
 if (!(priv->devs = virChrdevAlloc()))
 goto error;
 
-if (!(priv->blockjobs = virHashNew(virObjectFreeHashData)))
-goto error;
+priv->blockjobs = virHashNew(virObjectFreeHashData);
 
 /* agent commands block by default, user can choose different behavior */
 priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK;
-- 
2.31.1

[libvirt PATCH 15/38] qemuInteropFetchConfigs: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/qemu/qemu_interop_config.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index ea2afcc070..848e8f7381 100644
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -88,7 +88,7 @@ qemuInteropFetchConfigs(const char *name,
 char ***configs,
 bool privileged)
 {
-g_autoptr(GHashTable) files = NULL;
+g_autoptr(GHashTable) files = virHashNew(g_free);
 g_autofree char *homeConfig = NULL;
 g_autofree char *xdgConfig = NULL;
 g_autofree char *sysLocation = virFileBuildPath(QEMU_SYSTEM_LOCATION, 
name, NULL);
@@ -117,9 +117,6 @@ qemuInteropFetchConfigs(const char *name,
 homeConfig = g_strdup_printf("%s/qemu/%s", xdgConfig, name);
 }
 
-if (!(files = virHashNew(g_free)))
-return -1;
-
 if (qemuBuildFileList(files, sysLocation) < 0)
 return -1;
 
-- 
2.31.1

[libvirt PATCH 13/38] qemuBlockNodeNameGetBackingChain: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/qemu/qemu_block.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 2815eb54fa..8150241015 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -231,22 +231,16 @@ qemuBlockNodeNameGetBackingChain(virJSONValue *namednodes,
  virJSONValue *blockstats)
 {
 struct qemuBlockNodeNameGetBackingChainData data;
-g_autoptr(GHashTable) namednodestable = NULL;
-g_autoptr(GHashTable) disks = NULL;
+g_autoptr(GHashTable) namednodestable = virHashNew(virJSONValueHashFree);
+g_autoptr(GHashTable) disks = 
virHashNew(qemuBlockNodeNameBackingChainDataHashEntryFree);
 
 memset(&data, 0, sizeof(data));
 
-if (!(namednodestable = virHashNew(virJSONValueHashFree)))
-return NULL;
-
 if (virJSONValueArrayForeachSteal(namednodes,
   qemuBlockNamedNodesArrayToHash,
   namednodestable) < 0)
 return NULL;
 
-if (!(disks = virHashNew(qemuBlockNodeNameBackingChainDataHashEntryFree)))
-return NULL;
-
 data.nodenamestable = namednodestable;
 data.disks = disks;
 
-- 
2.31.1

[libvirt PATCH 10/38] qemusecuritymock: init_hash: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 tests/qemusecuritymock.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c
index 5daf27ccd7..87aadf564e 100644
--- a/tests/qemusecuritymock.c
+++ b/tests/qemusecuritymock.c
@@ -100,20 +100,9 @@ init_hash(void)
 if (xattr_paths)
 return;
 
-if (!(xattr_paths = virHashNew(g_free))) {
-fprintf(stderr, "Unable to create hash table for XATTR paths\n");
-abort();
-}
-
-if (!(chown_paths = virHashNew(g_free))) {
-fprintf(stderr, "Unable to create hash table for chowned paths\n");
-abort();
-}
-
-if (!(selinux_paths = virHashNew(g_free))) {
-fprintf(stderr, "Unable to create hash table for selinux labels\n");
-abort();
-}
+xattr_paths = virHashNew(g_free);
+chown_paths = virHashNew(g_free);
+selinux_paths = virHashNew(g_free);
 }
 
 
-- 
2.31.1

[libvirt PATCH 12/38] qemuBlockNodeNamesDetect: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/qemu/qemu_block.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 6627d044cd..2815eb54fa 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -365,10 +365,7 @@ qemuBlockNodeNamesDetect(virQEMUDriver *driver,
 GHashTable *
 qemuBlockGetNodeData(virJSONValue *data)
 {
-g_autoptr(GHashTable) nodedata = NULL;
-
-if (!(nodedata = virHashNew(virJSONValueHashFree)))
-return NULL;
+g_autoptr(GHashTable) nodedata = virHashNew(virJSONValueHashFree);
 
 if (virJSONValueArrayForeachSteal(data,
   qemuBlockNamedNodesArrayToHash, 
nodedata) < 0)
-- 
2.31.1

[libvirt PATCH 06/38] virNWFilterCreateVarsFrom: Remove superfluous `goto`s

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/nwfilter/nwfilter_gentech_driver.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c 
b/src/nwfilter/nwfilter_gentech_driver.c
index d17be401e6..a425285e8c 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -202,15 +202,12 @@ virNWFilterCreateVarsFrom(GHashTable *vars1,
 g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree);
 
 if (virNWFilterHashTablePutAll(vars1, res) < 0)
-goto error;
+return NULL;
 
 if (virNWFilterHashTablePutAll(vars2, res) < 0)
-goto error;
+return NULL;
 
 return g_steal_pointer(&res);
-
- error:
-return NULL;
 }
 
 
-- 
2.31.1

[libvirt PATCH 07/38] virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/nwfilter/nwfilter_gentech_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c 
b/src/nwfilter/nwfilter_gentech_driver.c
index a425285e8c..f98af5d513 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -261,8 +261,8 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def,
 ruleinst->chainPriority = def->chainPriority;
 ruleinst->def = rule;
 ruleinst->priority = rule->priority;
-if (!(ruleinst->vars = virHashNew(virNWFilterVarValueHashFree)))
-goto cleanup;
+ruleinst->vars = virHashNew(virNWFilterVarValueHashFree);
+
 if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0)
 goto cleanup;
 
-- 
2.31.1

[libvirt PATCH 08/38] ebiptablesApplyNewRules: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/nwfilter/nwfilter_ebiptables_driver.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c 
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 345562bab6..ec17d43c4e 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3338,9 +3338,6 @@ ebiptablesApplyNewRules(const char *ifname,
 size_t nsubchains = 0;
 int ret = -1;
 
-if (!chains_in_set || !chains_out_set)
-goto cleanup;
-
 if (nrules)
 qsort(rules, nrules, sizeof(rules[0]),
   virNWFilterRuleInstSortPtr);
-- 
2.31.1

[libvirt PATCH 02/38] virSystemdActivationNew: Use automatic memory management

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virsystemd.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 6417dc6ea7..f90c17e767 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -958,11 +958,10 @@ virSystemdActivationNew(virSystemdActivationMap *map,
 size_t nmap,
 int nfds)
 {
-virSystemdActivation *act;
+g_autoptr(virSystemdActivation) act = g_new0(virSystemdActivation, 1);
 const char *fdnames;
 
 VIR_DEBUG("Activated with %d FDs", nfds);
-act = g_new0(virSystemdActivation, 1);
 
 act->fds = virHashNew(virSystemdActivationEntryFree);
 
@@ -976,10 +975,9 @@ virSystemdActivationNew(virSystemdActivationMap *map,
 }
 
 VIR_DEBUG("Created activation object for %d FDs", nfds);
-return act;
+return g_steal_pointer(&act);
 
  error:
-virSystemdActivationFree(act);
 return NULL;
 }
 
-- 
2.31.1

[libvirt PATCH 05/38] virNWFilterCreateVarsFrom: Use automatic memory management

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/nwfilter/nwfilter_gentech_driver.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c 
b/src/nwfilter/nwfilter_gentech_driver.c
index dbb6b1f80e..d17be401e6 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -199,7 +199,7 @@ static GHashTable *
 virNWFilterCreateVarsFrom(GHashTable *vars1,
   GHashTable *vars2)
 {
-GHashTable *res = virHashNew(virNWFilterVarValueHashFree);
+g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree);
 
 if (virNWFilterHashTablePutAll(vars1, res) < 0)
 goto error;
@@ -207,10 +207,9 @@ virNWFilterCreateVarsFrom(GHashTable *vars1,
 if (virNWFilterHashTablePutAll(vars2, res) < 0)
 goto error;
 
-return res;
+return g_steal_pointer(&res);
 
  error:
-virHashFree(res);
 return NULL;
 }
 
-- 
2.31.1

[libvirt PATCH 03/38] virSystemdActivationNew: Remove superfluous `goto`s

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virsystemd.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index f90c17e767..99c92b6f52 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -968,17 +968,14 @@ virSystemdActivationNew(virSystemdActivationMap *map,
 fdnames = getenv("LISTEN_FDNAMES");
 if (fdnames) {
 if (virSystemdActivationInitFromNames(act, nfds, fdnames) < 0)
-goto error;
+return NULL;
 } else {
 if (virSystemdActivationInitFromMap(act, nfds, map, nmap) < 0)
-goto error;
+return NULL;
 }
 
 VIR_DEBUG("Created activation object for %d FDs", nfds);
 return g_steal_pointer(&act);
-
- error:
-return NULL;
 }
 
 
-- 
2.31.1

[libvirt PATCH 04/38] virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/nwfilter/nwfilter_gentech_driver.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c 
b/src/nwfilter/nwfilter_gentech_driver.c
index da4f71daf1..dbb6b1f80e 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -200,8 +200,6 @@ virNWFilterCreateVarsFrom(GHashTable *vars1,
   GHashTable *vars2)
 {
 GHashTable *res = virHashNew(virNWFilterVarValueHashFree);
-if (!res)
-return NULL;
 
 if (virNWFilterHashTablePutAll(vars1, res) < 0)
 goto error;
-- 
2.31.1

[libvirt PATCH 00/38] virHashNew refactorings - part VII

[Tim Wiederhake]

"virHashNew" cannot return NULL, yet we check for NULL in various places.

See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html.

Not split up further as per
https://listman.redhat.com/archives/libvir-list/2021-July/msg00575.html

Tim Wiederhake (38):
  virSystemdActivationNew: `virHashNew` cannot return NULL
  virSystemdActivationNew: Use automatic memory management
  virSystemdActivationNew: Remove superfluous `goto`s
  virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL
  virNWFilterCreateVarsFrom: Use automatic memory management
  virNWFilterCreateVarsFrom: Remove superfluous `goto`s
  virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL
  ebiptablesApplyNewRules: `virHashNew` cannot return NULL
  hypervCreateEmbeddedParam: `virHashNew` cannot return NULL
  qemusecuritymock: init_hash: `virHashNew` cannot return NULL
  libxlLoggerNew: `virHashNew` cannot return NULL
  qemuBlockNodeNamesDetect: `virHashNew` cannot return NULL
  qemuBlockNodeNameGetBackingChain: `virHashNew` cannot return NULL
  qemuDomainObjPrivateAlloc: `virHashNew` cannot return NULL
  qemuInteropFetchConfigs: `virHashNew` cannot return NULL
  virChrdevAlloc: `virHashNew` cannot return NULL
  virCloseCallbacksNew: `virHashNew` cannot return NULL
  virDomainCCWAddressSetCreate: `virHashNew` cannot return NULL
  virDomainDefBootOrderPostParse: `virHashNew` cannot return NULL
  virDomainDefValidateAliases: `virHashNew` cannot return NULL
  virDomainMomentObjListNew: `virHashNew` cannot return NULL
  virDomainObjListNew: `virHashNew` cannot return NULL
  virFileCacheNew: `virHashNew` cannot return NULL
  virHashAtomicNew: `virHashNew` cannot return NULL
  virInterfaceObjListNew: `virHashNew` cannot return NULL
  virLockDaemonNew: `virHashNew` cannot return NULL
  virLockDaemonNewPostExecRestart: `virHashNew` cannot return NULL
  virLockSpaceNew: `virHashNew` cannot return NULL
  virLockSpaceNewPostExecRestart: `virHashNew` cannot return NULL
  virNetDaemonNew: `virHashNew` cannot return NULL
  virNetworkObjListNew: `virHashNew` cannot return NULL
  virNetworkObjNew: `virHashNew` cannot return NULL
  virNodeDeviceObjListNew: `virHashNew` cannot return NULL
  virQEMUCapsProbeQMPHostCPU: `virHashNew` cannot return NULL
  virSecuritySELinuxLXCInitialize: `virHashNew` cannot return NULL
  virSecuritySELinuxQEMUInitialize: `virHashNew` cannot return NULL
  virStoragePoolObjListNew: `virHashNew` cannot return NULL
  virStorageVolObjListNew: `virHashNew` cannot return NULL

 src/conf/domain_addr.c|  7 +--
 src/conf/domain_conf.c|  5 +
 src/conf/domain_validate.c|  3 +--
 src/conf/virchrdev.c  |  6 +-
 src/conf/virdomainmomentobjlist.c |  4 
 src/conf/virdomainobjlist.c   |  8 ++--
 src/conf/virinterfaceobj.c|  5 +
 src/conf/virnetworkobj.c  |  8 ++--
 src/conf/virnodedeviceobj.c   |  5 +
 src/conf/virstorageobj.c  | 17 +
 src/hyperv/hyperv_wmi.c   |  6 +-
 src/hypervisor/virclosecallbacks.c|  4 
 src/libxl/libxl_logger.c  |  4 +---
 src/locking/lock_daemon.c |  6 ++
 src/nwfilter/nwfilter_ebiptables_driver.c |  3 ---
 src/nwfilter/nwfilter_gentech_driver.c| 18 ++
 src/qemu/qemu_block.c | 15 +++
 src/qemu/qemu_capabilities.c  |  3 +--
 src/qemu/qemu_domain.c|  3 +--
 src/qemu/qemu_interop_config.c|  5 +
 src/rpc/virnetdaemon.c|  3 +--
 src/security/security_selinux.c   |  6 ++
 src/util/virfilecache.c   |  7 +--
 src/util/virhash.c|  5 +
 src/util/virlockspace.c   |  6 ++
 src/util/virsystemd.c | 16 +---
 tests/qemusecuritymock.c  | 17 +++--
 27 files changed, 46 insertions(+), 149 deletions(-)

-- 
2.31.1

[libvirt PATCH 01/38] virSystemdActivationNew: `virHashNew` cannot return NULL

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/util/virsystemd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 96f81cd3fa..6417dc6ea7 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -964,8 +964,7 @@ virSystemdActivationNew(virSystemdActivationMap *map,
 VIR_DEBUG("Activated with %d FDs", nfds);
 act = g_new0(virSystemdActivation, 1);
 
-if (!(act->fds = virHashNew(virSystemdActivationEntryFree)))
-goto error;
+act->fds = virHashNew(virSystemdActivationEntryFree);
 
 fdnames = getenv("LISTEN_FDNAMES");
 if (fdnames) {
-- 
2.31.1

[PATCH] conf: Restore ctxt's node in right scope

[Zhenzhong Duan]

We just found  is ignored in our xml. Further debug
shows that ctxt's node pointer isn't restored in virDomainSecDefParseXML(),
which leads to parsing of remaining elements failed.

Signed-off-by: Zhenzhong Duan 
---
 src/conf/domain_conf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e05ea9ba88..13b7741eb8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14772,6 +14772,7 @@ static virDomainSecDef *
 virDomainSecDefParseXML(xmlNodePtr lsecNode,
 xmlXPathContextPtr ctxt)
 {
+VIR_XPATH_NODE_AUTORESTORE(ctxt)
 g_autoptr(virDomainSecDef) sec = g_new0(virDomainSecDef, 1);
 
 ctxt->node = lsecNode;
-- 
2.25.1

[PATCH 4/4] virshtest: add test for domjobinfo

[Luke Yue]

Signed-off-by: Luke Yue 
---
 tests/virshtest.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/virshtest.c b/tests/virshtest.c
index fe0c420958..88609af89c 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -257,6 +257,13 @@ static int testCompareDomControlInfoByName(const void 
*data G_GNUC_UNUSED)
 return testCompareOutputLit(exp, NULL, argv);
 }
 
+static int testCompareDomJobInfoByName(const void *data G_GNUC_UNUSED)
+{
+const char *const argv[] = { VIRSH_CUSTOM, "domjobinfo", "fc4", NULL };
+const char *exp = "Job type: None\n\n";
+return testCompareOutputLit(exp, NULL, argv);
+}
+
 struct testInfo {
 const char *const *argv;
 const char *result;
@@ -345,6 +352,10 @@ mymain(void)
testCompareDomControlInfoByName, NULL) != 0)
 ret = -1;
 
+if (virTestRun("virsh domjobinfo (by name)",
+   testCompareDomJobInfoByName, NULL) != 0)
+ret = -1;
+
 /* It's a bit awkward listing result before argument, but that's a
  * limitation of C99 vararg macros.  */
 # define DO_TEST(i, result, ...) \
-- 
2.32.0

[PATCH 3/4] test_driver: Implement virDomainAbortJob

[Luke Yue]

As the job should be running between 0-1 seconds, so if the API is
triggered between 0-1 seconds, it will set time to 1, so that
the job status will be completed. Otherwise, there is no job active and
report error.

Signed-off-by: Luke Yue 
---
 src/test/test_driver.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4af8ce42f2..b31ab95101 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2744,6 +2744,34 @@ testDomainGetJobStats(virDomainPtr domain,
 return ret;
 }
 
+static int
+testDomainAbortJob(virDomainPtr dom)
+{
+virDomainObj *vm;
+int ret = -1;
+testDomainObjPrivate *priv;
+
+if (!(vm = testDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+priv = vm->privateData;
+
+if (priv->seconds >= 1) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("no job is active on the domain"));
+} else {
+priv->seconds = 1;
+ret = 0;
+}
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
 static int
 testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
 virTypedParameterPtr *params G_GNUC_UNUSED,
@@ -9625,6 +9653,7 @@ static virHypervisorDriver testHypervisorDriver = {
 .domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
 .domainGetJobInfo = testDomainGetJobInfo, /* 7.6.0 */
 .domainGetJobStats = testDomainGetJobStats, /* 7.6.0 */
+.domainAbortJob = testDomainAbortJob, /* 7.6.0 */
 
 .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
 .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
-- 
2.32.0

[PATCH 2/4] test_driver: Implement virDomainGetJobStats

[Luke Yue]

Signed-off-by: Luke Yue 
---
 src/test/test_driver.c | 90 ++
 1 file changed, 90 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 29b19d80f0..4af8ce42f2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2654,6 +2654,95 @@ testDomainGetJobInfo(virDomainPtr dom,
 return ret;
 }
 
+static int
+testDomainJobInfoToParams(virDomainJobInfoPtr info,
+  int *type,
+  virTypedParameterPtr *params,
+  int *nparams)
+{
+virTypedParameterPtr par = NULL;
+int maxpar = 0;
+int npar = 0;
+
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DATA_TOTAL,
+info->dataTotal) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DATA_PROCESSED,
+info->dataProcessed) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DATA_REMAINING,
+info->dataRemaining) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_TIME_ELAPSED,
+info->timeElapsed) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_TIME_REMAINING,
+info->timeRemaining) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_TOTAL,
+info->memTotal) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_PROCESSED,
+info->memProcessed) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_REMAINING,
+info->memRemaining) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DISK_TOTAL,
+info->fileTotal) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DISK_PROCESSED,
+info->fileProcessed) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DISK_REMAINING,
+info->fileRemaining) < 0)
+goto error;
+
+*type = info->type;
+*params = par;
+*nparams = npar;
+return 0;
+
+ error:
+virTypedParamsFree(par, npar);
+return -1;
+}
+
+static int
+testDomainGetJobStats(virDomainPtr domain,
+  int *type,
+  virTypedParameterPtr *params,
+  int *nparams,
+  unsigned int flags)
+{
+virDomainJobInfo jobInfo;
+virDomainObj *dom;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(dom = testDomObjFromDomain(domain)))
+return -1;
+
+if (testDomainGetJobInfoImpl(dom, &jobInfo) < 0)
+goto cleanup;
+
+if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
+*type = VIR_DOMAIN_JOB_NONE;
+*params = NULL;
+*nparams = 0;
+ret = 0;
+goto cleanup;
+}
+
+ret = testDomainJobInfoToParams(&jobInfo, type, params, nparams);
+
+ cleanup:
+virDomainObjEndAPI(&dom);
+
+return ret;
+}
 
 static int
 testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
@@ -9535,6 +9624,7 @@ static virHypervisorDriver testHypervisorDriver = {
 .domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
 .domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
 .domainGetJobInfo = testDomainGetJobInfo, /* 7.6.0 */
+.domainGetJobStats = testDomainGetJobStats, /* 7.6.0 */
 
 .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
 .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
-- 
2.32.0

[PATCH 0/4] Implement some job related APIs for test driver

[Luke Yue]

Luke Yue (4):
  test_driver: Implement virDomainGetJobInfo
  test_driver: Implement virDomainGetJobStats
  test_driver: Implement virDomainAbortJob
  virshtest: add test for domjobinfo

 src/test/test_driver.c | 170 +
 tests/virshtest.c  |  11 +++
 2 files changed, 181 insertions(+)

-- 
2.32.0

[PATCH 1/4] test_driver: Implement virDomainGetJobInfo

[Luke Yue]

As in testDomainGetControlInfo, a background job should be running
between 0-1 seconds, so make the testDomainGetJobInfo consistent
with it.

Signed-off-by: Luke Yue 
---
 src/test/test_driver.c | 51 ++
 1 file changed, 51 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 892dc978f2..29b19d80f0 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2604,6 +2604,56 @@ testDomainGetOSType(virDomainPtr dom G_GNUC_UNUSED)
 return ret;
 }
 
+static int
+testDomainGetJobInfoImpl(virDomainObj *dom,
+ virDomainJobInfoPtr info)
+{
+testDomainObjPrivate *priv = dom->privateData;
+
+memset(info, 0, sizeof(*info));
+
+if (priv->seconds < 1) {
+info->type = VIR_DOMAIN_JOB_BOUNDED;
+info->dataTotal = 1;
+info->dataProcessed = priv->seconds;
+info->dataRemaining = 1 - priv->seconds;
+info->timeElapsed = priv->seconds;
+info->timeRemaining = 1 - priv->seconds;
+info->memTotal = 1048576;
+info->memProcessed = 1048576 - priv->seconds;
+info->memRemaining = priv->seconds;
+info->fileTotal = 2097152;
+info->fileProcessed = 2097152 - priv->seconds;
+info->fileRemaining = priv->seconds;
+} else if (priv->seconds < 3 && priv->seconds >= 1) {
+info->type = VIR_DOMAIN_JOB_COMPLETED;
+} else {
+info->type = VIR_DOMAIN_JOB_NONE;
+}
+
+return 0;
+}
+
+static int
+testDomainGetJobInfo(virDomainPtr dom,
+ virDomainJobInfoPtr info)
+{
+virDomainObj *vm;
+int ret = -1;
+
+if (!(vm = testDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+ret = testDomainGetJobInfoImpl(vm, info);
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
 
 static int
 testDomainGetLaunchSecurityInfo(virDomainPtr domain G_GNUC_UNUSED,
@@ -9484,6 +9534,7 @@ static virHypervisorDriver testHypervisorDriver = {
 .domainMemoryPeek = testDomainMemoryPeek, /* 5.4.0 */
 .domainGetBlockInfo = testDomainGetBlockInfo, /* 5.7.0 */
 .domainSetLifecycleAction = testDomainSetLifecycleAction, /* 5.7.0 */
+.domainGetJobInfo = testDomainGetJobInfo, /* 7.6.0 */
 
 .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
 .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.4 */
-- 
2.32.0

[PATCH v3 13/13] docs/deprecated: deprecate passing plugin args through `arg=`

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 docs/system/deprecated.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e2e0090878..7ae6f1f727 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -126,6 +126,13 @@ other options have been processed.  This will either have 
no effect (if
 if they were not given.  The property is therefore useless and should not be
 specified.
 
+Plugin argument passing through ``arg=`` (since 6.1)
+
+
+Passing TCG plugins arguments through ``arg=`` is redundant is makes the
+command-line less readable, especially when the argument itself consist of a
+name and a value, e.g. ``-plugin plugin_name,arg="arg_name=arg_value"``.
+Therefore, the usage of ``arg`` is redundant.
 
 QEMU Machine Protocol (QMP) commands
 
-- 
2.25.1