[PATCH V2] tests: Fix qemuxml2xmltest with audio driver defined in env

[Jim Fehlig]

If QEMU_AUDIO_DRV is defined in the build host environment, several tests
in qemuxml2xmltest fail.

$ env | grep -i audio
AUDIODRIVER=pulseaudio
QEMU_AUDIO_DRV=pa
SDL_AUDIODRIVER=pulse

An example test failure with the above environment

907) QEMU XML-2-XML-active video-virtio-gpu-sdl-gl
In 'libvirt/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml':
Offset 1244
Expect [v]
Actual [audio id='1' type='pulseaudio'/>

---

v2: Also scrub SDL_AUDIODRIVER

 tests/qemuxml2xmltest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 137f1871af..823dd40f50 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -198,6 +198,8 @@ mymain(void)
  * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
  * values for these envvars */
 g_setenv("PATH", "/bin", TRUE);
+g_unsetenv("QEMU_AUDIO_DRV");
+g_unsetenv("SDL_AUDIODRIVER");
 
 DO_TEST("minimal", NONE);
 DO_TEST_CAPS_LATEST("genid");
-- 
2.30.1

[PATCH] tests: Fix qemuxml2xmltest with audio driver defined in env

[Jim Fehlig]

If QEMU_AUDIO_DRV is defined in the build host environment, several tests
in qemuxml2xmltest fail.

$ env | grep -i audio
AUDIODRIVER=pulseaudio
QEMU_AUDIO_DRV=pa
SDL_AUDIODRIVER=pulse

An example test failure with the above environment

907) QEMU XML-2-XML-active video-virtio-gpu-sdl-gl
In 'libvirt/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml':
Offset 1244
Expect [v]
Actual [audio id='1' type='pulseaudio'/>

---

On IRC Daniel suggested scrubbing QEM_AUDIO_DRV and SDL_AUDIODRIVER.
Only QEM_AUDIO_DRV was needed to fix the test failure, but I can scrub
the other one too if that's preferred.

 tests/qemuxml2xmltest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 137f1871af..aff6ae9175 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -198,6 +198,7 @@ mymain(void)
  * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
  * values for these envvars */
 g_setenv("PATH", "/bin", TRUE);
+g_unsetenv("QEMU_AUDIO_DRV");
 
 DO_TEST("minimal", NONE);
 DO_TEST_CAPS_LATEST("genid");
-- 
2.30.1

Re: [PATCH RFC 0/3] Add checkpoint/restore support to LXC using CRIU

[Martin Kletzander]

On Thu, Apr 01, 2021 at 10:16:36AM -0300, Julio Faracco wrote:

Hi Michal and Martin,

Thanks for your reply.
Just an explanation. I'm not interested directly in developing this
specific feature.
If there is a GSoC student addressed to this... Excellent.
I'm interested in developing snapshot and container migration which
unfortunately requires this feature.
Unless you have another opinion.



That student was working on it, but that was 5 years ago as Michal said.
I'm afraid that it's either you or nobody who does that =)


--
Julio Faracco

Em qui., 1 de abr. de 2021 às 07:33, Michal Privoznik
 escreveu:


On 4/1/21 12:01 AM, Martin Kletzander wrote:
> On Sat, Feb 27, 2021 at 01:14:29AM -0300, Julio Faracco wrote:
>> Hi guys,
>>
>
> Hi and sorry for not replying earlier.
>

Yeah, sorry. I have this marked for review and yet still haven't done so.

>> I marked this series as RFC to discuss some points. I'm interested in
>> enhancing this specific part of LXC. So, some questions that I would
>> like to hear as a feedback from community:
>> 1. I decided to use a tar to compress all CRIU img files into a single
>> file. Any other suggestions?
>> 2. If no is the answer to question above, is there a consensus on
>> preferring to use command line calls or libraries? I would like to use
>> libtar for instance. I personally think that this approach is ugly.
>> Not sure if I'm able to do that. The same for CRIU.
>
> I remember that for CRIU, back when we were trying to do that, the issue
> was that the commands were not atomic, did not properly report error
> messages and maybe something more along the lines.  Either there was no
> library interface or it was not MT-safe, basically there were couple of
> issues like that which we were not able to deal with.
>
> I do not really remember all the details.  Maybe Michal does as I think
> he suggested the idea back then.  I Cc'd him.  In the worst scenario we
> will need to figure this all out again ;)

IIRC the main problem was that we wanted CRIU to be able to send its
data over a TCP connection. Back then, when a GSoC student was looking
at this, CRIU was only able to store data into a file (or even multiple
files in a directory?) and wasn't able to create server/client
connection. Maybe this has changed since then? If not, then we can use
tar, sure. And to transfer data we can use so called tunnelled
migration, where the migration stream is sent over libvirt connection
rather than directly to the other side (because then we would have to
have nc or similar involved).

https://libvirt.org/migration.html#transporttunnel

Another issue was that it couldn't handle all namespaces (but I'm not
certain - it was 5 years ago).

But let me find some time and review patches.

Michal





signature.asc
Description: PGP signature

Re: [libvirt PATCH] downloads.html: Add a link to GPG key used signing releases

[Ján Tomko]

On a Thursday in 2021, Jiri Denemark wrote:

While the key is available on public GPG key servers, having it locally
at https://libvirt.org/sources/gpg_key.asc is even better.



I don't remember where but I think someone was trying to find the
key used to sign libvirt-glib. Also, Pavel uses his key to sign
libvirt-dbus releases.

We could reflect that in the naming scheme to put their keys there too.
Or put all the keys in gpg_keys.asc, like GnuPG does:
https://gnupg.org/signature_key.html

I also noticed that we have empty folders there (csharp, go, ruby, rust) and 
that
the 'old' release folder was not "updated" in a while.


Signed-off-by: Jiri Denemark 
---
docs/downloads.html.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/downloads.html.in b/docs/downloads.html.in
index ca14b3ecba..90a0cf7717 100644
--- a/docs/downloads.html.in
+++ b/docs/downloads.html.in
@@ -608,7 +608,9 @@ git clone git://libvirt.org/[module name].git
  on this project site are signed with a GPG signature. You should always
  verify the package signature before using the source to compile binary
  packages. The following key is currently used to generate the GPG
-  signatures:
+  signatures and it can be
+  https://libvirt.org/sources/gpg_key.asc;>downloaded from 
this
+  site or from public GPG key servers:


Reviewed-by: Ján Tomko 

Jano




pub  4096R/10084C9C 2020-07-20 Jiří Denemark jdene...@redhat.com
--
2.31.1



signature.asc
Description: PGP signature

Re: [PATCH V2] tests: Fix qemuxml2xmltest with audio driver defined in env

[Daniel P . Berrangé]

On Thu, Apr 01, 2021 at 11:44:59AM -0600, Jim Fehlig wrote:
> If QEMU_AUDIO_DRV is defined in the build host environment, several tests
> in qemuxml2xmltest fail.
> 
> $ env | grep -i audio
> AUDIODRIVER=pulseaudio
> QEMU_AUDIO_DRV=pa
> SDL_AUDIODRIVER=pulse
> 
> An example test failure with the above environment
> 
> 907) QEMU XML-2-XML-active video-virtio-gpu-sdl-gl
> In 'libvirt/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml':
> Offset 1244
> Expect [v]
> Actual [audio id='1' type='pulseaudio'/>
>  
> Scrub QEMU_AUDIO_DRV from the environment before executing the tests in
> qemuxml2xmltest. SDL_AUDIODRIVER also needs scrubbed since it will be
> examined if QEMU_AUDIO_DRV=sdl.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> v2: Also scrub SDL_AUDIODRIVER
> 
>  tests/qemuxml2xmltest.c | 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] downloads.html: Add a link to GPG key used signing releases

[Andrea Bolognani]

On Thu, 2021-04-01 at 17:36 +0200, Jiri Denemark wrote:
> While the key is available on public GPG key servers, having it locally
> at https://libvirt.org/sources/gpg_key.asc is even better.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  docs/downloads.html.in | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

I love the idea, but I would like to suggest a slightly alternative
implementation of it:

diff --git a/docs/downloads.html.in b/docs/downloads.html.in
index ca14b3ecba..0187062cef 100644
--- a/docs/downloads.html.in
+++ b/docs/downloads.html.in
@@ -615,6 +615,12 @@ pub  4096R/10084C9C 2020-07-20 Jiří Denemark 
jdene...@redhat.com
 Fingerprint=453B 6531 0595 5628 5547  1199 CA68 BE80 1008 4C9C
 

+
+  It can be downloaded from
+  https://libvirt.org/sources/gpg_key.asc;>this site or from
+  public GPG key servers.
+
+
 
   Releases prior to libvirt-6.6 were signed with the following GPG key:
 

What do you think?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] tests: Fix qemuxml2xmltest with audio driver defined in env

[Daniel P . Berrangé]

On Thu, Apr 01, 2021 at 11:10:28AM -0600, Jim Fehlig wrote:
> If QEMU_AUDIO_DRV is defined in the build host environment, several tests
> in qemuxml2xmltest fail.
> 
> $ env | grep -i audio
> AUDIODRIVER=pulseaudio
> QEMU_AUDIO_DRV=pa
> SDL_AUDIODRIVER=pulse
> 
> An example test failure with the above environment
> 
> 907) QEMU XML-2-XML-active video-virtio-gpu-sdl-gl
> In 'libvirt/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml':
> Offset 1244
> Expect [v]
> Actual [audio id='1' type='pulseaudio'/>
>  
> Scrub QEMU_AUDIO_DRV from the environment before executing the tests in
> qemuxml2xmltest.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> On IRC Daniel suggested scrubbing QEM_AUDIO_DRV and SDL_AUDIODRIVER.
> Only QEM_AUDIO_DRV was needed to fix the test failure, but I can scrub
> the other one too if that's preferred.

I think we should do both as qemu_domain.c will look at SDL_AUDIODRIVER
if $QEMU_AUDIO_DRV == sdl

> 
>  tests/qemuxml2xmltest.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 137f1871af..aff6ae9175 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -198,6 +198,7 @@ mymain(void)
>   * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
>   * values for these envvars */
>  g_setenv("PATH", "/bin", TRUE);
> +g_unsetenv("QEMU_AUDIO_DRV");
>  
>  DO_TEST("minimal", NONE);
>  DO_TEST_CAPS_LATEST("genid");
> -- 
> 2.30.1
> 
> 

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] downloads.html: Add a link to GPG key used signing releases

[Jiri Denemark]

While the key is available on public GPG key servers, having it locally
at https://libvirt.org/sources/gpg_key.asc is even better.

Signed-off-by: Jiri Denemark 
---
 docs/downloads.html.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/downloads.html.in b/docs/downloads.html.in
index ca14b3ecba..90a0cf7717 100644
--- a/docs/downloads.html.in
+++ b/docs/downloads.html.in
@@ -608,7 +608,9 @@ git clone git://libvirt.org/[module name].git
   on this project site are signed with a GPG signature. You should always
   verify the package signature before using the source to compile binary
   packages. The following key is currently used to generate the GPG
-  signatures:
+  signatures and it can be
+  https://libvirt.org/sources/gpg_key.asc;>downloaded from 
this
+  site or from public GPG key servers:
 
 
 pub  4096R/10084C9C 2020-07-20 Jiří Denemark jdene...@redhat.com
-- 
2.31.1

Re: [libvirt PATCH v6 00/30] Add support for persistent mediated devices

[Jonathon Jongsma]

On Wed, 31 Mar 2021 16:00:48 +0200
Erik Skultety  wrote:

> On Fri, Mar 26, 2021 at 11:47:56AM -0500, Jonathon Jongsma wrote:
> > This patch series follows the previously-merged series which added
> > support for transient mediated devices. This series expands mdev
> > support to include persistent device definitions. Again, it relies
> > on mdevctl as the backend.
> > 
> > It follows the common libvirt pattern of APIs by adding the
> > following new APIs for node devices:
> > - virNodeDeviceDefineXML() - defines a persistent device
> > - virNodeDeviceUndefine() - undefines a persistent device
> > - virNodeDeviceCreate() - starts a previously-defined device
> > 
> > It also adds virsh commands mapping to these new APIs:
> > nodedev-define, nodedev-undefine, and nodedev-start.
> > 
> > Since we rely on mdevctl for the definition of mediated devices, we
> > need a way to stay up-to-date with devices that are defined by
> > mdevctl (outside of libvirt).  The method for staying up-to-date is
> > currently a little bit crude due to the fact that mdevctl does not
> > emit any events when new devices are added or removed. As a
> > workaround, we create a file monitor for the mdevctl config
> > directory and re-query mdevctl when we detect changes within that
> > directory. In the future, mdevctl may introduce a more elegant
> > solution.
> > 
> > Changes in v6:
> >  - rebase to git master again
> >  - remove typedefs for various *Ptr types, since they're now
> > discouraged in libvirt.  
> 
> Okay, so I think I ACKed all of the patches now (if not, let me know
> which one I missed). I tested the patches, found 3 leaks, one of them
> I mentioned here, 2 were related to the code but not a direct result
> of this series I believe, so I'll tackle them some other time as a
> follow up. Overall, I think we're good to push this starting with the
> 7.3.0 cycle.

I think everything else has been acked.

> 
> Now, in v4 I promised to share my adjustments I made on top of your
> code. Some of them you already handled yourself in v6, so I rebased
> and performed a bunch of changes, so here they are:
> https://gitlab.com/eskultety/libvirt/-/commits/mdev-adjustments
> 
> Note, that I only split them into multiple patches so that they're
> easier to read, but I'm very well aware that probably none of them
> cannot be compiled on its own (I didn't bother to be that thorough),
> it's just to give you an idea what I've failed to express with words
> until we came to this v6. Please let me know your opinion on the
> changes so that: a) I can send it as a *proper* follow up series
> b) you can incorporate some of the changes into your series
> 

Thanks for that. I think the changes look reasonable, and I think the
benefits outweigh the drawbacks. I have a few changes I'd like to make
to your commits. See the top 3 commits at [1] for details. Would you
like me to just incorporate your stuff into my original series, or keep
them as separate commits on top of my series?

Jonathon


[1] https://gitlab.com/jjongsma/libvirt/-/commits/mdevctl-adjustments/

[libvir PATCH] run: fix flake8 violations

[Daniel P . Berrangé]

Two blank lines are needed either side of functions.

Comments must have a single space character immediately after
the "#".

The unused exception variable can be removed.

Signed-off-by: Daniel P. Berrangé 
---

I dropped the ball by not running syntax check on the 'run' script
changes. Pushed this fixup for the style problems.

 run.in | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/run.in b/run.in
index b710ba87fb..b778505a33 100644
--- a/run.in
+++ b/run.in
@@ -16,7 +16,7 @@
 # License along with this library; If not, see
 # .
 
-#--
+# --
 #
 # With this script you can run libvirt programs without needing to
 # install them first.  You just have to do for example:
@@ -38,7 +38,7 @@
 #
 #   sudo ./run virsh list --all
 #
-#--
+# --
 
 import os
 import os.path
@@ -46,6 +46,7 @@ import random
 import sys
 import subprocess
 
+
 # Function to intelligently prepend a path to an environment variable.
 # See https://stackoverflow.com/a/9631350
 def prepend(env, varname, extradir):
@@ -54,6 +55,7 @@ def prepend(env, varname, extradir):
 else:
 env[varname] = extradir
 
+
 here = "@abs_builddir@"
 
 if len(sys.argv) < 2:
@@ -93,29 +95,36 @@ modular_daemons = [
 "virtxend",
 ]
 
+
 def is_modular_daemon(name):
 return name in modular_daemons
 
+
 def is_monolithic_daemon(name):
 return name == "libvirtd"
 
+
 def is_systemd_host():
 if os.getuid() != 0:
 return False
 return os.path.exists("/run/systemd/system")
 
+
 def daemon_units(name):
 return [name + suffix for suffix in [
 ".service", ".socket", "-ro.socket", "-admin.socket"]]
 
+
 def is_unit_active(name):
 ret = subprocess.call(["systemctl", "is-active", "-q", name])
 return ret == 0
 
+
 def change_unit(name, action):
 ret = subprocess.call(["systemctl", action, "-q", name])
 return ret == 0
 
+
 try_stop_units = []
 if is_systemd_host():
 name = os.path.basename(prog)
@@ -152,7 +161,7 @@ else:
 
 print("Running %s..." % prog)
 ret = subprocess.call([prog] + args, env=env)
-except KeyboardInterrupt as ex:
+except KeyboardInterrupt:
 pass
 finally:
 print("Re-starting original systemd units...")
-- 
2.30.2

[PATCH 12/39] util: virlog: Remove pointless 'cleanup' labels

[Peter Krempa]

Previous refactors left empty cleanup labels. Remove them.

Signed-off-by: Peter Krempa 
---
 src/util/virlog.c | 47 +--
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 4782f6f27d..004792db01 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1473,7 +1473,7 @@ virLogParseOutput(const char *src)
 if (!(tokens = virStringSplitCount(src, ":", 0, )) || count < 2) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Malformed format for output '%s'"), src);
-goto cleanup;
+return NULL;
 }

 if (virStrToLong_uip(tokens[0], NULL, 10, ) < 0 ||
@@ -1481,14 +1481,14 @@ virLogParseOutput(const char *src)
 virReportError(VIR_ERR_INVALID_ARG,
_("Invalid priority '%s' for output '%s'"),
tokens[0], src);
-goto cleanup;
+return NULL;
 }

 if ((dest = virLogDestinationTypeFromString(tokens[1])) < 0) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Invalid destination '%s' for output '%s'"),
tokens[1], src);
-goto cleanup;
+return NULL;
 }

 if (((dest == VIR_LOG_TO_STDERR ||
@@ -1498,7 +1498,7 @@ virLogParseOutput(const char *src)
 virReportError(VIR_ERR_INVALID_ARG,
_("Output '%s' does not meet the format requirements "
  "for destination type '%s'"), src, tokens[1]);
-goto cleanup;
+return NULL;
 }

 switch ((virLogDestination) dest) {
@@ -1512,7 +1512,7 @@ virLogParseOutput(const char *src)
 break;
 case VIR_LOG_TO_FILE:
 if (virFileAbsPath(tokens[2], ) < 0)
-goto cleanup;
+return NULL;
 ret = virLogNewOutputToFile(prio, abspath);
 VIR_FREE(abspath);
 break;
@@ -1525,7 +1525,6 @@ virLogParseOutput(const char *src)
 break;
 }

- cleanup:
 return ret;
 }

@@ -1557,7 +1556,6 @@ virLogParseOutput(const char *src)
 virLogFilterPtr
 virLogParseFilter(const char *src)
 {
-virLogFilterPtr ret = NULL;
 size_t count = 0;
 virLogPriority prio;
 g_auto(GStrv) tokens = NULL;
@@ -1569,7 +1567,7 @@ virLogParseFilter(const char *src)
 if (!(tokens = virStringSplitCount(src, ":", 0, )) || count != 2) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Malformed format for filter '%s'"), src);
-goto cleanup;
+return NULL;
 }

 if (virStrToLong_uip(tokens[0], NULL, 10, ) < 0 ||
@@ -1577,7 +1575,7 @@ virLogParseFilter(const char *src)
 virReportError(VIR_ERR_INVALID_ARG,
_("Invalid priority '%s' for filter '%s'"),
tokens[0], src);
-goto cleanup;
+return NULL;
 }

 match = tokens[1];
@@ -1592,15 +1590,10 @@ virLogParseFilter(const char *src)
 if (!*match) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Invalid match string '%s'"), tokens[1]);
-
-goto cleanup;
+return NULL;
 }

-if (!(ret = virLogFilterNew(match, prio)))
-goto cleanup;
-
- cleanup:
-return ret;
+return virLogFilterNew(match, prio);
 }

 /**
@@ -1617,7 +1610,6 @@ virLogParseFilter(const char *src)
 int
 virLogParseOutputs(const char *src, virLogOutputPtr **outputs)
 {
-int ret = -1;
 int at = -1;
 size_t noutputs = 0;
 size_t i, count;
@@ -1628,7 +1620,7 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 VIR_DEBUG("outputs=%s", src);

 if (!(strings = virStringSplitCount(src, " ", 0, )))
-goto cleanup;
+return -1;

 for (i = 0; i < count; i++) {
 /* virStringSplit may return empty strings */
@@ -1636,7 +1628,7 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 continue;

 if (!(output = virLogParseOutput(strings[i])))
-goto cleanup;
+return -1;

 /* let's check if a duplicate output does not already exist in which
  * case we need to replace it with its last occurrence, however, rather
@@ -1647,7 +1639,7 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 at = virLogFindOutput(list, noutputs, output->dest, output->name);
 if (VIR_APPEND_ELEMENT(list, noutputs, output) < 0) {
 virLogOutputFree(output);
-goto cleanup;
+return -1;
 }
 if (at >= 0) {
 virLogOutputFree(list[at]);
@@ -1655,10 +1647,8 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 }
 }

-ret = noutputs;
 *outputs = g_steal_pointer();
- cleanup:
-return ret;
+return noutputs;
 }

 /**
@@ -1677,7 +1667,6 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 int
 virLogParseFilters(const char *src, virLogFilterPtr **filters)
 {
-int 

[PATCH 20/39] virshParseRateStr: Refactor cleanup

[Peter Krempa]

Use g_auto for the string list and remove 'ret' and 'cleanup:'.

Signed-off-by: Peter Krempa 
---
 tools/virsh-domain.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 86de4255fa..37be298809 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -848,7 +848,7 @@ static const vshCmdOptDef opts_attach_interface[] = {
 *tok[index] != '\0' && \
 virStrToLong_ullp(tok[index], NULL, 10, >name) < 0) { \
 vshError(ctl, _("field '%s' is malformed"), #name); \
-goto cleanup; \
+return -1; \
 } \
 } while (0)

@@ -857,16 +857,15 @@ virshParseRateStr(vshControl *ctl,
   const char *rateStr,
   virNetDevBandwidthRatePtr rate)
 {
-char **tok = NULL;
+g_auto(GStrv) tok = NULL;
 size_t ntok;
-int ret = -1;

 if (!(tok = virStringSplitCount(rateStr, ",", 0, )))
 return -1;

 if (ntok > 4) {
 vshError(ctl, _("Rate string '%s' has too many fields"), rateStr);
-goto cleanup;
+return -1;
 }

 VIRSH_PARSE_RATE_FIELD(0, average);
@@ -874,10 +873,7 @@ virshParseRateStr(vshControl *ctl,
 VIRSH_PARSE_RATE_FIELD(2, burst);
 VIRSH_PARSE_RATE_FIELD(3, floor);

-ret = 0;
- cleanup:
-g_strfreev(tok);
-return ret;
+return 0;
 }

 #undef VIRSH_PARSE_RATE_FIELD
-- 
2.29.2

[PATCH 38/39] util: virstring: Remove the virStringSplitCount wrapper funcion

[Peter Krempa]

Callers which need the count of elements now count it in place.

Signed-off-by: Peter Krempa 
---
 src/libvirt_private.syms |  1 -
 src/util/virstring.c | 20 
 src/util/virstring.h |  6 --
 3 files changed, 27 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9208db2056..68cdb92453 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3260,7 +3260,6 @@ virStringReplace;
 virStringSearch;
 virStringSortCompare;
 virStringSortRevCompare;
-virStringSplitCount;
 virStringStripControlChars;
 virStringStripIPv6Brackets;
 virStringStripSuffix;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 7749eb2db5..a2c07e5c17 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -35,26 +35,6 @@

 VIR_LOG_INIT("util.string");

-/**
- * virStringSplitCount:
- *
- * A wrapper for g_strsplit which provides number of elements of the split
- * string.
- */
-char **
-virStringSplitCount(const char *string,
-const char *delim,
-size_t max_tokens,
-size_t *tokcount)
-{
-GStrv ret = g_strsplit(string, delim, max_tokens);
-
-*tokcount = g_strv_length(ret);
-
-return ret;
-}
-
-
 /**
  * virStringListMerge:
  * @dst: a NULL-terminated array of strings to expand
diff --git a/src/util/virstring.h b/src/util/virstring.h
index e688495574..7cc1d8c55f 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -22,12 +22,6 @@

 #define VIR_INT64_STR_BUFLEN 21

-char **virStringSplitCount(const char *string,
-   const char *delim,
-   size_t max_tokens,
-   size_t *tokcount)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
-
 int virStringListMerge(char ***dst,
char ***src);

-- 
2.29.2

[PATCH 25/39] storage: zfs: Use g_strsplit instead of virStringSplitCount

[Peter Krempa]

Both instances just check the length once. Replicate that faithfully.

Signed-off-by: Peter Krempa 
---
 src/storage/storage_backend_zfs.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_backend_zfs.c 
b/src/storage/storage_backend_zfs.c
index 6fed25cf4d..c36deb9dac 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -96,7 +96,6 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool,
  const char *volume_string)
 {
 int ret = -1;
-size_t count;
 char *vol_name;
 bool is_new_vol = false;
 virStorageVolDefPtr volume = NULL;
@@ -104,10 +103,10 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool,
 g_auto(GStrv) tokens = NULL;
 char *tmp;

-if (!(tokens = virStringSplitCount(volume_string, "\t", 0, )))
+if (!(tokens = g_strsplit(volume_string, "\t", 0)))
 return -1;

-if (count != 3)
+if (g_strv_length(tokens) != 3)
 goto cleanup;

 vol_name = tokens[0];
@@ -246,16 +245,15 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool 
G_GNUC_UNUSED)

 for (i = 0; lines[i]; i++) {
 g_auto(GStrv) tokens = NULL;
-size_t count;
 char *prop_name;

 if (STREQ(lines[i], ""))
 continue;

-if (!(tokens = virStringSplitCount(lines[i], "\t", 0, )))
+if (!(tokens = g_strsplit(lines[i], "\t", 0)))
 goto cleanup;

-if (count != 4)
+if (g_strv_length(tokens) != 4)
 continue;

 prop_name = tokens[1];
-- 
2.29.2

[PATCH 26/39] virResctrlGetCacheInfo: Restrict variable scope and use automatic freeing

[Peter Krempa]

Move variables into the loop which uses them and use automatic freeing
for temporarily allocated variables.

Signed-off-by: Peter Krempa 
---
 src/util/virresctrl.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 8458db96bc..fbc8e9c766 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -526,18 +526,19 @@ static int
 virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
DIR *dirp)
 {
-char *endptr = NULL;
-char *tmp_str = NULL;
-int ret = -1;
 int rv = -1;
-int type = 0;
+int ret = -1;
 struct dirent *ent = NULL;
-unsigned int level = 0;
-virBitmapPtr tmp_map = NULL;
-virResctrlInfoPerLevelPtr i_level = NULL;
-virResctrlInfoPerTypePtr i_type = NULL;

 while ((rv = virDirRead(dirp, , SYSFS_RESCTRL_PATH "/info")) > 0) {
+g_autofree char *cbm_mask_str = NULL;
+g_autoptr(virBitmap) cbm_mask_map = NULL;
+char *endptr = NULL;
+int type = 0;
+unsigned int level = 0;
+virResctrlInfoPerLevelPtr i_level = NULL;
+g_autofree virResctrlInfoPerTypePtr i_type = NULL;
+
 VIR_DEBUG("Parsing info type '%s'", ent->d_name);
 if (ent->d_name[0] != 'L')
 continue;
@@ -570,7 +571,7 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
 goto cleanup;
 }

-rv = virFileReadValueString(_str,
+rv = virFileReadValueString(_mask_str,
 SYSFS_RESCTRL_PATH
 "/info/%s/cbm_mask",
 ent->d_name);
@@ -585,19 +586,15 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
 if (rv < 0)
 goto cleanup;

-virStringTrimOptionalNewline(tmp_str);
+virStringTrimOptionalNewline(cbm_mask_str);

-tmp_map = virBitmapNewString(tmp_str);
-VIR_FREE(tmp_str);
-if (!tmp_map) {
+if (!(cbm_mask_map = virBitmapNewString(cbm_mask_str))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse cbm_mask from resctrl cache info"));
 goto cleanup;
 }

-i_type->bits = virBitmapCountBits(tmp_map);
-virBitmapFree(tmp_map);
-tmp_map = NULL;
+i_type->bits = virBitmapCountBits(cbm_mask_map);

 rv = virFileReadValueUint(_type->min_cbm_bits,
   SYSFS_RESCTRL_PATH "/info/%s/min_cbm_bits",
@@ -635,7 +632,6 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,

 ret = 0;
  cleanup:
-VIR_FREE(i_type);
 return ret;
 }

-- 
2.29.2

[PATCH 37/39] virVMXParseConfig: Replace virStringSplitCount by g_strsplit

[Peter Krempa]

Remove the last usage of virStringSplitCount

Signed-off-by: Peter Krempa 
---
 src/vmx/vmx.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 0b12b5dd7d..7832fd143e 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1574,18 +1574,17 @@ virVMXParseConfig(virVMXContext *ctx,
 if (sched_cpu_affinity != NULL && STRCASENEQ(sched_cpu_affinity, "all")) {
 g_auto(GStrv) afflist = NULL;
 char **aff;
-size_t naffs;

 def->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN);

-if (!(afflist = virStringSplitCount(sched_cpu_affinity, ",", 0, 
)))
+if (!(afflist = g_strsplit(sched_cpu_affinity, ",", 0)))
 goto cleanup;

-if (naffs < numvcpus) {
+if (g_strv_length(afflist) < numvcpus) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Expecting VMX entry 'sched.cpu.affinity' to 
contain "
  "at least as many values as 'numvcpus' (%lld) but 
"
- "found only %zu value(s)"), numvcpus, naffs);
+ "found only %u value(s)"), numvcpus, 
g_strv_length(afflist));
 goto cleanup;
 }

-- 
2.29.2

[PATCH 24/39] virStorageBackendZFSRefreshPool: Reduce scope of 'tokens'

[Peter Krempa]

Declare it in the loop that actually uses it.

Signed-off-by: Peter Krempa 
---
 src/storage/storage_backend_zfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_zfs.c 
b/src/storage/storage_backend_zfs.c
index cab1fd1637..6fed25cf4d 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -216,7 +216,6 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool 
G_GNUC_UNUSED)
 size_t i;
 g_autoptr(virCommand) cmd = NULL;
 g_auto(GStrv) lines = NULL;
-g_auto(GStrv) tokens = NULL;
 g_autofree char *name = g_strdup(def->source.name);
 char *tmp;

@@ -246,13 +245,13 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool 
G_GNUC_UNUSED)
 goto cleanup;

 for (i = 0; lines[i]; i++) {
+g_auto(GStrv) tokens = NULL;
 size_t count;
 char *prop_name;

 if (STREQ(lines[i], ""))
 continue;

-g_strfreev(tokens);
 if (!(tokens = virStringSplitCount(lines[i], "\t", 0, )))
 goto cleanup;

-- 
2.29.2

[PATCH 27/39] virResctrlAllocNewFromInfo: Restrict variable scope and use automatic freeing

[Peter Krempa]

Move variables into the loop which uses them and use automatic freeing
for temporarily allocated variables.

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

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index fbc8e9c766..5e7c391524 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -1811,27 +1811,26 @@ static virResctrlAllocPtr
 virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
 {
 size_t i = 0;
-size_t j = 0;
-size_t k = 0;
 virResctrlAllocPtr ret = virResctrlAllocNew();
-virBitmapPtr mask = NULL;

 if (!ret)
 return NULL;

 for (i = 0; i < info->nlevels; i++) {
 virResctrlInfoPerLevelPtr i_level = info->levels[i];
+size_t j = 0;

 if (!i_level)
 continue;

 for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) {
 virResctrlInfoPerTypePtr i_type = i_level->types[j];
+g_autoptr(virBitmap) mask = NULL;
+size_t k = 0;

 if (!i_type)
 continue;

-virBitmapFree(mask);
 mask = virBitmapNew(i_type->bits);
 virBitmapSetAll(mask);

@@ -1856,7 +1855,6 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
 }

  cleanup:
-virBitmapFree(mask);
 return ret;
  error:
 virObjectUnref(ret);
-- 
2.29.2

[PATCH 15/39] virLogParseFilter: Replace virStringSplitCount by g_strsplit

[Peter Krempa]

We don't really need the count.

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

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 57eb0de538..8a4cfbfdc2 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1556,7 +1556,6 @@ virLogParseOutput(const char *src)
 virLogFilterPtr
 virLogParseFilter(const char *src)
 {
-size_t count = 0;
 virLogPriority prio;
 g_auto(GStrv) tokens = NULL;
 char *match = NULL;
@@ -1564,7 +1563,8 @@ virLogParseFilter(const char *src)
 VIR_DEBUG("filter=%s", src);

 /* split our format prio:match_str to tokens and parse them individually */
-if (!(tokens = virStringSplitCount(src, ":", 0, )) || count != 2) {
+if (!(tokens = g_strsplit(src, ":", 0)) ||
+!tokens[0] || !tokens[1]) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Malformed format for filter '%s'"), src);
 return NULL;
-- 
2.29.2

[PATCH 33/39] xenParsePCI: Replace virStringSplitCount by g_strsplit

[Peter Krempa]

Count the number of elements in place just for the check.

Signed-off-by: Peter Krempa 
---
 src/libxl/xen_common.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 835f48ec42..49a816add2 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -372,7 +372,6 @@ xenParsePCI(char *entry)
 virDomainHostdevDefPtr hostdev = NULL;
 g_auto(GStrv) tokens = NULL;
 g_auto(GStrv) options = NULL;
-size_t ntokens = 0;
 size_t nexttoken = 0;
 char *str;
 char *nextstr;
@@ -383,11 +382,11 @@ xenParsePCI(char *entry)
 virTristateBool filtered = VIR_TRISTATE_BOOL_ABSENT;

 /* pci=['00:1b.0',':00:13.0,permissive=1'] */
-if (!(tokens = virStringSplitCount(entry, ":", 3, )))
+if (!(tokens = g_strsplit(entry, ":", 3)))
 return NULL;

 /* domain */
-if (ntokens == 3) {
+if (g_strv_length(tokens) == 3) {
 if (virStrToLong_i(tokens[nexttoken], NULL, 16, ) < 0)
 return NULL;
 nexttoken++;
-- 
2.29.2

[PATCH 31/39] util: virresctrl: Remove empty 'cleanup' sections

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/util/virresctrl.c | 111 +++---
 1 file changed, 39 insertions(+), 72 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 6d8e669852..73b0061937 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -527,7 +527,6 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
DIR *dirp)
 {
 int rv = -1;
-int ret = -1;
 struct dirent *ent = NULL;

 while ((rv = virDirRead(dirp, , SYSFS_RESCTRL_PATH "/info")) > 0) {
@@ -568,7 +567,7 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
  ent->d_name);
 } else if (rv < 0) {
 /* Other failures are fatal, so just quit */
-goto cleanup;
+return -1;
 }

 rv = virFileReadValueString(_mask_str,
@@ -584,14 +583,14 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
_("Cannot get cbm_mask from resctrl cache info"));
 }
 if (rv < 0)
-goto cleanup;
+return -1;

 virStringTrimOptionalNewline(cbm_mask_str);

 if (!(cbm_mask_map = virBitmapNewString(cbm_mask_str))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot parse cbm_mask from resctrl cache info"));
-goto cleanup;
+return -1;
 }

 i_type->bits = virBitmapCountBits(cbm_mask_map);
@@ -603,7 +602,7 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot get min_cbm_bits from resctrl cache 
info"));
 if (rv < 0)
-goto cleanup;
+return -1;

 if (resctrl->nlevels <= level)
 VIR_EXPAND_N(resctrl->levels, resctrl->nlevels,
@@ -624,22 +623,19 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Duplicate cache type in resctrl for level %u"),
level);
-goto cleanup;
+return -1;
 }

 i_level->types[type] = g_steal_pointer(_type);
 }

-ret = 0;
- cleanup:
-return ret;
+return 0;
 }


 static int
 virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
 {
-int ret = -1;
 int rv = -1;
 g_autofree virResctrlInfoMemBWPtr i_membw = NULL;

@@ -652,11 +648,10 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr 
resctrl)
  * probably memory bandwidth allocation unsupported */
 VIR_INFO("The path '" SYSFS_RESCTRL_PATH "/info/MB/bandwidth_gran'"
  "does not exist");
-ret = 0;
-goto cleanup;
+return 0;
 } else if (rv < 0) {
 /* Other failures are fatal, so just quit */
-goto cleanup;
+return -1;
 }

 rv = virFileReadValueUint(_membw->min_bandwidth,
@@ -669,7 +664,7 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
_("Cannot get min bandwidth from resctrl memory info"));
 }
 if (rv < 0)
-goto cleanup;
+return -1;

 rv = virFileReadValueUint(_membw->max_allocation,
   SYSFS_RESCTRL_PATH "/info/MB/num_closids");
@@ -679,12 +674,10 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr 
resctrl)
_("Cannot get max allocation from resctrl memory 
info"));
 }
 if (rv < 0)
-goto cleanup;
+return -1;

 resctrl->membw_info = g_steal_pointer(_membw);
-ret = 0;
- cleanup:
-return ret;
+return 0;
 }


@@ -702,7 +695,6 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
 static int
 virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 {
-int ret = -1;
 int rv = -1;
 g_autofree char *featurestr = NULL;
 g_auto(GStrv) features = NULL;
@@ -721,11 +713,10 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
  * monitor unsupported */
 VIR_INFO("The file '" SYSFS_RESCTRL_PATH "/info/L3_MON/num_rmids' "
  "does not exist");
-ret = 0;
-goto cleanup;
+return 0;
 } else if (rv < 0) {
 /* Other failures are fatal, so just quit */
-goto cleanup;
+return -1;
 }

 rv = virFileReadValueUint(_monitor->cache_reuse_threshold,
@@ -737,7 +728,7 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 VIR_DEBUG("File '" SYSFS_RESCTRL_PATH
   "/info/L3_MON/max_threshold_occupancy' does not exist");
 } else if (rv < 0) {
-goto cleanup;
+return -1;
 }

 rv = virFileReadValueString(,
@@ -747,14 +738,14 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot get mon_features from resctrl"));
 if (rv < 0)
-goto cleanup;
+return -1;

 if 

[PATCH 36/39] virSystemdActivationInitFromNames: Replace virStringSplit by g_strsplit

[Peter Krempa]

While the code invokes the string list length calculation twice, it
happens only on error path, which by itself should never happen.

Signed-off-by: Peter Krempa 
---
 src/util/virsystemd.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index cf22edaa0a..718d24dfc5 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -754,19 +754,18 @@ virSystemdActivationInitFromNames(virSystemdActivationPtr 
act,
 {
 g_auto(GStrv) fdnamelistptr = NULL;
 char **fdnamelist;
-size_t nfdnames;
 size_t i;
 int nextfd = STDERR_FILENO + 1;

 VIR_DEBUG("FD names %s", fdnames);

-if (!(fdnamelistptr = virStringSplitCount(fdnames, ":", 0, )))
+if (!(fdnamelistptr = g_strsplit(fdnames, ":", 0)))
 goto error;

-if (nfdnames != nfds) {
+if (g_strv_length(fdnamelistptr) != nfds) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Expecting %d FD names but got %zu"),
-   nfds, nfdnames);
+   _("Expecting %d FD names but got %u"),
+   nfds, g_strv_length(fdnamelistptr));
 goto error;
 }

-- 
2.29.2

[PATCH 32/39] util: virresctrl: Use g_strsplit instead of virStringSplitCount

[Peter Krempa]

In 3 of 4 instances the code didn't even need the count of the elements.

Signed-off-by: Peter Krempa 
---
 src/util/virresctrl.c | 38 --
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 73b0061937..7891bc7411 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -697,8 +697,6 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 {
 int rv = -1;
 g_autofree char *featurestr = NULL;
-g_auto(GStrv) features = NULL;
-size_t nfeatures = 0;
 g_autofree virResctrlInfoMongrpPtr info_monitor = NULL;

 info_monitor = g_new0(virResctrlInfoMongrp, 1);
@@ -748,11 +746,10 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 return -1;
 }

-features = virStringSplitCount(featurestr, "\n", 0, );
-VIR_DEBUG("Resctrl supported %zd monitoring features", nfeatures);
+info_monitor->features = g_strsplit(featurestr, "\n", 0);
+info_monitor->nfeatures = g_strv_length(info_monitor->features);
+VIR_DEBUG("Resctrl supported %zd monitoring features", 
info_monitor->nfeatures);

-info_monitor->nfeatures = nfeatures;
-info_monitor->features = g_steal_pointer();
 resctrl->monitor_info = g_steal_pointer(_monitor);

 return 0;
@@ -1466,9 +1463,8 @@ virResctrlAllocParseMemoryBandwidthLine(virResctrlInfoPtr 
resctrl,
 char *line)
 {
 g_auto(GStrv) mbs = NULL;
+GStrv next;
 char *tmp = NULL;
-size_t nmbs = 0;
-size_t i;

 /* For no reason there can be spaces */
 virSkipSpaces((const char **) );
@@ -1493,9 +1489,9 @@ virResctrlAllocParseMemoryBandwidthLine(virResctrlInfoPtr 
resctrl,
 return 0;
 tmp++;

-mbs = virStringSplitCount(tmp, ";", 0, );
-for (i = 0; i < nmbs; i++) {
-if (virResctrlAllocParseProcessMemoryBandwidth(resctrl, alloc, mbs[i]) 
< 0)
+mbs = g_strsplit(tmp, ";", 0);
+for (next = mbs; *next; next++) {
+if (virResctrlAllocParseProcessMemoryBandwidth(resctrl, alloc, *next) 
< 0)
 return -1;
 }

@@ -1620,11 +1616,10 @@ virResctrlAllocParseCacheLine(virResctrlInfoPtr resctrl,
   char *line)
 {
 g_auto(GStrv) caches = NULL;
+GStrv next;
 char *tmp = NULL;
 unsigned int level = 0;
 int type = -1;
-size_t ncaches = 0;
-size_t i = 0;

 /* For no reason there can be spaces */
 virSkipSpaces((const char **) );
@@ -1656,12 +1651,12 @@ virResctrlAllocParseCacheLine(virResctrlInfoPtr resctrl,
 return -1;
 }

-caches = virStringSplitCount(tmp, ";", 0, );
+caches = g_strsplit(tmp, ";", 0);
 if (!caches)
 return 0;

-for (i = 0; i < ncaches; i++) {
-if (virResctrlAllocParseProcessCache(resctrl, alloc, level, type, 
caches[i]) < 0)
+for (next = caches; *next; next++) {
+if (virResctrlAllocParseProcessCache(resctrl, alloc, level, type, 
*next) < 0)
 return -1;
 }

@@ -1675,14 +1670,13 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl,
  const char *schemata)
 {
 g_auto(GStrv) lines = NULL;
-size_t nlines = 0;
-size_t i = 0;
+GStrv next;

-lines = virStringSplitCount(schemata, "\n", 0, );
-for (i = 0; i < nlines; i++) {
-if (virResctrlAllocParseCacheLine(resctrl, alloc, lines[i]) < 0)
+lines = g_strsplit(schemata, "\n", 0);
+for (next = lines; *next; next++) {
+if (virResctrlAllocParseCacheLine(resctrl, alloc, *next) < 0)
 return -1;
-if (virResctrlAllocParseMemoryBandwidthLine(resctrl, alloc, lines[i]) 
< 0)
+if (virResctrlAllocParseMemoryBandwidthLine(resctrl, alloc, *next) < 0)
 return -1;
 }

-- 
2.29.2

[PATCH 35/39] openvzParseBarrierLimit: Rework string handling

[Peter Krempa]

Use g_strsplit instead of virStringSplitCount and automatically free the
temporary string list.

Signed-off-by: Peter Krempa 
---
 src/openvz/openvz_conf.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 836885af18..cf29ce30ad 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -123,26 +123,21 @@ openvzParseBarrierLimit(const char* value,
 unsigned long long *barrier,
 unsigned long long *limit)
 {
-char **tmp = NULL;
-size_t ntmp = 0;
-int ret = -1;
+g_auto(GStrv) tmp = NULL;

-if (!(tmp = virStringSplitCount(value, ":", 0, )))
-goto error;
+if (!(tmp = g_strsplit(value, ":", 0)))
+return -1;

-if (ntmp != 2)
-goto error;
+if (g_strv_length(tmp) != 2)
+return -1;

 if (barrier && virStrToLong_ull(tmp[0], NULL, 10, barrier) < 0)
-goto error;
+return -1;

 if (limit && virStrToLong_ull(tmp[1], NULL, 10, limit) < 0)
-goto error;
+return -1;

-ret = 0;
- error:
-virStringListFreeCount(tmp, ntmp);
-return ret;
+return 0;
 }


-- 
2.29.2

[PATCH 22/39] virStorageSourceParseBackingJSONUriCookies: Use g_strsplit instead of virStringSplitCount

[Peter Krempa]

Count the elements after splitting the string.

Signed-off-by: Peter Krempa 
---
 src/storage_file/storage_source_backingstore.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/storage_file/storage_source_backingstore.c 
b/src/storage_file/storage_source_backingstore.c
index bac5a043e5..9a67d64fd6 100644
--- a/src/storage_file/storage_source_backingstore.c
+++ b/src/storage_file/storage_source_backingstore.c
@@ -483,7 +483,6 @@ 
virStorageSourceParseBackingJSONUriCookies(virStorageSourcePtr src,
 {
 const char *cookiestr;
 g_auto(GStrv) cookies = NULL;
-size_t ncookies = 0;
 size_t i;

 if (!virJSONValueObjectHasKey(json, "cookie"))
@@ -496,13 +495,13 @@ 
virStorageSourceParseBackingJSONUriCookies(virStorageSourcePtr src,
 return -1;
 }

-if (!(cookies = virStringSplitCount(cookiestr, ";", 0, )))
+if (!(cookies = g_strsplit(cookiestr, ";", 0)))
 return -1;

-src->cookies = g_new0(virStorageNetCookieDefPtr, ncookies);
-src->ncookies = ncookies;
+src->ncookies = g_strv_length(cookies);
+src->cookies = g_new0(virStorageNetCookieDefPtr, src->ncookies);

-for (i = 0; i < ncookies; i++) {
+for (i = 0; i < src->ncookies; i++) {
 char *cookiename = cookies[i];
 char *cookievalue;

-- 
2.29.2

[PATCH 39/39] tests: string: Remove pointless test for virStringListFreeCount

[Peter Krempa]

It's way more useful to run valgrind against the rest of the code than
this test to see whether virStringListFreeCount works. Remove the test.

Signed-off-by: Peter Krempa 
---
 tests/virstringtest.c | 23 ---
 1 file changed, 23 deletions(-)

diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index 82e8e2106f..83b883524d 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -392,24 +392,6 @@ testStringToDouble(const void *opaque)
 return ret;
 }

-/* The point of this test is to check whether all members of the array are
- * freed. The test has to be checked using valgrind. */
-static int
-testVirStringListFreeCount(const void *opaque G_GNUC_UNUSED)
-{
-char **list;
-
-list = g_new0(char *, 4);
-
-list[0] = g_strdup("test1");
-list[2] = g_strdup("test2");
-list[3] = g_strdup("test3");
-
-virStringListFreeCount(list, 4);
-
-return 0;
-}
-

 struct testStripData {
 const char *string;
@@ -718,11 +700,6 @@ mymain(void)
 NULL,
 3.141592653589793238462643383279502884197169399375105);

-/* test virStringListFreeCount */
-if (virTestRun("virStringListFreeCount", testVirStringListFreeCount,
-   NULL) < 0)
-ret = -1;
-
 #define TEST_STRIP_IPV6_BRACKETS(str, res) \
 do { \
 struct testStripData stripData = { \
-- 
2.29.2

[PATCH 17/39] virshParseEventStr: Use g_strsplit and automatic memory freeing

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 tools/virsh-domain.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a778421b66..86de4255fa 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9342,24 +9342,21 @@ virshParseEventStr(const char *event,
int *nparams,
int *maxparams)
 {
-char **tok = NULL;
-size_t i, ntok;
-int ret = -1;
+g_auto(GStrv) tok = NULL;
+GStrv next;

-if (!(tok = virStringSplitCount(event, ",", 0, )))
+if (!(tok = g_strsplit(event, ",", 0)))
 return -1;

-for (i = 0; i < ntok; i++) {
-if ((*tok[i] != '\0') &&
-virTypedParamsAddBoolean(params, nparams,
- maxparams, tok[i], state) < 0)
-goto cleanup;
+for (next = tok; *next; next++) {
+if (*next[0] == '\0')
+continue;
+
+if (virTypedParamsAddBoolean(params, nparams, maxparams, *next, state) 
< 0)
+return -1;
 }

-ret = 0;
- cleanup:
-g_strfreev(tok);
-return ret;
+return 0;
 }

 static void
-- 
2.29.2

[PATCH 34/39] xenParseXLVnuma: Replace virStringSplitCount by g_strsplit

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/libxl/xen_xl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index 65cd26bb21..e8a75979d4 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -523,9 +523,11 @@ xenParseXLVnuma(virConfPtr conf,
 tmp = g_strdup(vtoken);

 g_strfreev(token);
-if (!(token = virStringSplitCount(tmp, ",", 0, 
)))
+if (!(token = g_strsplit(tmp, ",", 0)))
 goto cleanup;

+ndistances = g_strv_length(token);
+
 if (ndistances != nr_nodes) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("vnuma pnode %d configured '%s' 
(count %zu) doesn't fit the number of specified vnodes %zu"),
-- 
2.29.2

[PATCH 05/39] virStorageFileBackendGlusterPriv: Remove 'canonpaht'

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/storage_file/storage_file_backend_gluster.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/storage_file/storage_file_backend_gluster.c 
b/src/storage_file/storage_file_backend_gluster.c
index 252eb523af..0cd4cf9f62 100644
--- a/src/storage_file/storage_file_backend_gluster.c
+++ b/src/storage_file/storage_file_backend_gluster.c
@@ -41,7 +41,6 @@ typedef virStorageFileBackendGlusterPriv 
*virStorageFileBackendGlusterPrivPtr;

 struct _virStorageFileBackendGlusterPriv {
 glfs_t *vol;
-char *canonpath;
 };

 static void
@@ -55,7 +54,6 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)

 if (priv->vol)
 glfs_fini(priv->vol);
-VIR_FREE(priv->canonpath);

 VIR_FREE(priv);
 drv->priv = NULL;
-- 
2.29.2

[PATCH 14/39] virLogParseFilters: Refactor string list handling

[Peter Krempa]

Rewrite the code to remove the need to calculate the string list count.

Signed-off-by: Peter Krempa 
---
 src/util/virlog.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 56059f62f1..57eb0de538 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1668,22 +1668,22 @@ int
 virLogParseFilters(const char *src, virLogFilterPtr **filters)
 {
 size_t nfilters = 0;
-size_t i, count;
 g_auto(GStrv) strings = NULL;
+GStrv next;
 virLogFilterPtr filter = NULL;
 virLogFilterPtr *list = NULL;

 VIR_DEBUG("filters=%s", src);

-if (!(strings = virStringSplitCount(src, " ", 0, )))
+if (!(strings = g_strsplit(src, " ", 0)))
 return -1;

-for (i = 0; i < count; i++) {
+for (next = strings; *next; next++) {
 /* virStringSplit may return empty strings */
-if (STREQ(strings[i], ""))
+if (STREQ(*next, ""))
 continue;

-if (!(filter = virLogParseFilter(strings[i])))
+if (!(filter = virLogParseFilter(*next)))
 return -1;

 if (VIR_APPEND_ELEMENT(list, nfilters, filter)) {
-- 
2.29.2

[PATCH 28/39] virResctrlAllocNewFromInfo: Use g_autoptr for 'ret'

[Peter Krempa]

Remove 'cleanup' and 'error' labels by switching 'ret' to automatic
pointer and stealing it in the return statement.

Signed-off-by: Peter Krempa 
---
 src/util/virresctrl.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 5e7c391524..c66cf4b087 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -1811,7 +1811,7 @@ static virResctrlAllocPtr
 virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
 {
 size_t i = 0;
-virResctrlAllocPtr ret = virResctrlAllocNew();
+g_autoptr(virResctrlAlloc) ret = virResctrlAllocNew();

 if (!ret)
 return NULL;
@@ -1836,7 +1836,7 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)

 for (k = 0; k <= i_type->max_cache_id; k++) {
 if (virResctrlAllocUpdateMask(ret, i, j, k, mask) < 0)
-goto error;
+return NULL;
 }
 }
 }
@@ -1854,12 +1854,7 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
 }
 }

- cleanup:
-return ret;
- error:
-virObjectUnref(ret);
-ret = NULL;
-goto cleanup;
+return g_steal_pointer();
 }

 /*
-- 
2.29.2

[PATCH 30/39] util: virresctrl: Use automatic memory freeing

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/util/virresctrl.c | 53 ++-
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 53c202f99f..6d8e669852 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -641,7 +641,7 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
 {
 int ret = -1;
 int rv = -1;
-virResctrlInfoMemBWPtr i_membw = NULL;
+g_autofree virResctrlInfoMemBWPtr i_membw = NULL;

 /* query memory bandwidth allocation info */
 i_membw = g_new0(virResctrlInfoMemBW, 1);
@@ -684,7 +684,6 @@ virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
 resctrl->membw_info = g_steal_pointer(_membw);
 ret = 0;
  cleanup:
-VIR_FREE(i_membw);
 return ret;
 }

@@ -705,10 +704,10 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
 {
 int ret = -1;
 int rv = -1;
-char *featurestr = NULL;
-char **features = NULL;
+g_autofree char *featurestr = NULL;
+g_auto(GStrv) features = NULL;
 size_t nfeatures = 0;
-virResctrlInfoMongrpPtr info_monitor = NULL;
+g_autofree virResctrlInfoMongrpPtr info_monitor = NULL;

 info_monitor = g_new0(virResctrlInfoMongrp, 1);

@@ -767,9 +766,6 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)

 ret = 0;
  cleanup:
-VIR_FREE(featurestr);
-g_strfreev(features);
-VIR_FREE(info_monitor);
 return ret;
 }

@@ -1480,7 +1476,7 @@ virResctrlAllocParseMemoryBandwidthLine(virResctrlInfoPtr 
resctrl,
 virResctrlAllocPtr alloc,
 char *line)
 {
-char **mbs = NULL;
+g_auto(GStrv) mbs = NULL;
 char *tmp = NULL;
 size_t nmbs = 0;
 size_t i;
@@ -1517,7 +1513,6 @@ virResctrlAllocParseMemoryBandwidthLine(virResctrlInfoPtr 
resctrl,

 ret = 0;
  cleanup:
-g_strfreev(mbs);
 return ret;
 }

@@ -1595,7 +1590,7 @@ virResctrlAllocParseProcessCache(virResctrlInfoPtr 
resctrl,
 {
 char *tmp = strchr(cache, '=');
 unsigned int cache_id = 0;
-virBitmapPtr mask = NULL;
+g_autoptr(virBitmap) mask = NULL;
 int ret = -1;

 if (!tmp)
@@ -1632,7 +1627,6 @@ virResctrlAllocParseProcessCache(virResctrlInfoPtr 
resctrl,

 ret = 0;
  cleanup:
-virBitmapFree(mask);
 return ret;
 }

@@ -1642,7 +1636,7 @@ virResctrlAllocParseCacheLine(virResctrlInfoPtr resctrl,
   virResctrlAllocPtr alloc,
   char *line)
 {
-char **caches = NULL;
+g_auto(GStrv) caches = NULL;
 char *tmp = NULL;
 unsigned int level = 0;
 int type = -1;
@@ -1691,7 +1685,6 @@ virResctrlAllocParseCacheLine(virResctrlInfoPtr resctrl,

 ret = 0;
  cleanup:
-g_strfreev(caches);
 return ret;
 }

@@ -1701,7 +1694,7 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl,
  virResctrlAllocPtr alloc,
  const char *schemata)
 {
-char **lines = NULL;
+g_auto(GStrv) lines = NULL;
 size_t nlines = 0;
 size_t i = 0;
 int ret = -1;
@@ -1717,7 +1710,6 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl,

 ret = 0;
  cleanup:
-g_strfreev(lines);
 return ret;
 }

@@ -1945,7 +1937,7 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc,
 {
 unsigned long long *size = alloc->levels[level]->types[type]->sizes[cache];
 virBitmapPtr a_mask = NULL;
-virBitmapPtr f_mask = NULL;
+g_autoptr(virBitmap) f_mask = NULL;
 unsigned long long need_bits;
 size_t i = 0;
 ssize_t pos = -1;
@@ -2049,7 +2041,6 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc,

 ret = 0;
  cleanup:
-virBitmapFree(a_mask);
 return ret;
 }

@@ -2185,8 +2176,8 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
 {
 int ret = -1;
 unsigned int level = 0;
-virResctrlAllocPtr alloc_free = NULL;
-virResctrlAllocPtr alloc_default = NULL;
+g_autoptr(virResctrlAlloc) alloc_free = NULL;
+g_autoptr(virResctrlAlloc) alloc_default = NULL;

 alloc_free = virResctrlAllocGetUnused(resctrl);
 if (!alloc_free)
@@ -2251,8 +2242,6 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,

 ret = 0;
  cleanup:
-virObjectUnref(alloc_free);
-virObjectUnref(alloc_default);
 return ret;
 }

@@ -2328,8 +2317,8 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl,
   virResctrlAllocPtr alloc,
   const char *machinename)
 {
-char *schemata_path = NULL;
-char *alloc_str = NULL;
+g_autofree char *schemata_path = NULL;
+g_autofree char *alloc_str = NULL;
 int ret = -1;
 int lockfd = -1;

@@ -2377,8 +2366,6 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl,
 ret = 0;
  cleanup:
 virResctrlUnlock(lockfd);
-VIR_FREE(alloc_str);
-VIR_FREE(schemata_path);
 return ret;
 }

@@ -2387,8 +2374,8 @@ static int
 virResctrlAddPID(const char *path,
  pid_t pid)

[PATCH 11/39] util: virlog: Use g_auto(GStrv) instead of g_strfreev in cleanup section

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/util/virlog.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 934c96915b..4782f6f27d 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1459,7 +1459,7 @@ virLogOutputPtr
 virLogParseOutput(const char *src)
 {
 virLogOutputPtr ret = NULL;
-char **tokens = NULL;
+g_auto(GStrv) tokens = NULL;
 char *abspath = NULL;
 size_t count = 0;
 virLogPriority prio;
@@ -1526,7 +1526,6 @@ virLogParseOutput(const char *src)
 }

  cleanup:
-g_strfreev(tokens);
 return ret;
 }

@@ -1561,7 +1560,7 @@ virLogParseFilter(const char *src)
 virLogFilterPtr ret = NULL;
 size_t count = 0;
 virLogPriority prio;
-char **tokens = NULL;
+g_auto(GStrv) tokens = NULL;
 char *match = NULL;

 VIR_DEBUG("filter=%s", src);
@@ -1601,7 +1600,6 @@ virLogParseFilter(const char *src)
 goto cleanup;

  cleanup:
-g_strfreev(tokens);
 return ret;
 }

@@ -1623,7 +1621,7 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 int at = -1;
 size_t noutputs = 0;
 size_t i, count;
-char **strings = NULL;
+g_auto(GStrv) strings = NULL;
 virLogOutputPtr output = NULL;
 virLogOutputPtr *list = NULL;

@@ -1660,7 +1658,6 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 ret = noutputs;
 *outputs = g_steal_pointer();
  cleanup:
-g_strfreev(strings);
 return ret;
 }

@@ -1683,7 +1680,7 @@ virLogParseFilters(const char *src, virLogFilterPtr 
**filters)
 int ret = -1;
 size_t nfilters = 0;
 size_t i, count;
-char **strings = NULL;
+g_auto(GStrv) strings = NULL;
 virLogFilterPtr filter = NULL;
 virLogFilterPtr *list = NULL;

@@ -1709,7 +1706,6 @@ virLogParseFilters(const char *src, virLogFilterPtr 
**filters)
 ret = nfilters;
 *filters = g_steal_pointer();
  cleanup:
-g_strfreev(strings);
 return ret;
 }

-- 
2.29.2

[PATCH 03/39] Remove virStorageSourceGetUniqueIdentifier file backend API

[Peter Krempa]

The API isn't used any more.

Signed-off-by: Peter Krempa 
---
 src/libvirt_private.syms  |  1 -
 src/storage_file/storage_file_backend.h   |  4 --
 src/storage_file/storage_file_backend_fs.c| 24 ---
 .../storage_file_backend_gluster.c| 71 ---
 src/storage_file/storage_source.c | 37 +-
 5 files changed, 1 insertion(+), 136 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cb9fe7c80a..62ccda467f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1713,7 +1713,6 @@ virStorageSourceGetMetadata;
 virStorageSourceGetMetadataFromBuf;
 virStorageSourceGetMetadataFromFD;
 virStorageSourceGetRelativeBackingPath;
-virStorageSourceGetUniqueIdentifier;
 virStorageSourceInit;
 virStorageSourceInitAs;
 virStorageSourceNewFromBacking;
diff --git a/src/storage_file/storage_file_backend.h 
b/src/storage_file/storage_file_backend.h
index ecf5883a55..8ad579a8db 100644
--- a/src/storage_file/storage_file_backend.h
+++ b/src/storage_file/storage_file_backend.h
@@ -60,9 +60,6 @@ typedef ssize_t
  size_t len,
  char **buf);

-typedef const char *
-(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);
-
 typedef int
 (*virStorageFileBackendAccess)(virStorageSourcePtr src,
int mode);
@@ -88,7 +85,6 @@ struct _virStorageFileBackend {
 virStorageFileBackendInit backendInit;
 virStorageFileBackendDeinit backendDeinit;
 virStorageFileBackendRead storageFileRead;
-virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier;

 /* The following group of callbacks is expected to set errno
  * and return -1 on error. No libvirt error shall be reported */
diff --git a/src/storage_file/storage_file_backend_fs.c 
b/src/storage_file/storage_file_backend_fs.c
index 7b114fdeb0..f34ffd5fc8 100644
--- a/src/storage_file/storage_file_backend_fs.c
+++ b/src/storage_file/storage_file_backend_fs.c
@@ -147,24 +147,6 @@ virStorageFileBackendFileRead(virStorageSourcePtr src,
 }


-static const char *
-virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
-{
-virStorageDriverDataPtr drv = src->drv;
-virStorageFileBackendFsPrivPtr priv = drv->priv;
-
-if (!priv->canonpath) {
-if (!(priv->canonpath = virFileCanonicalizePath(src->path))) {
-virReportSystemError(errno, _("can't canonicalize path '%s'"),
- src->path);
-return NULL;
-}
-}
-
-return priv->canonpath;
-}
-
-
 static int
 virStorageFileBackendFileAccess(virStorageSourcePtr src,
 int mode)
@@ -197,8 +179,6 @@ virStorageFileBackend virStorageFileBackendFile = {
 .storageFileRead = virStorageFileBackendFileRead,
 .storageFileAccess = virStorageFileBackendFileAccess,
 .storageFileChown = virStorageFileBackendFileChown,
-
-.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


@@ -212,8 +192,6 @@ virStorageFileBackend virStorageFileBackendBlock = {
 .storageFileRead = virStorageFileBackendFileRead,
 .storageFileAccess = virStorageFileBackendFileAccess,
 .storageFileChown = virStorageFileBackendFileChown,
-
-.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


@@ -225,8 +203,6 @@ virStorageFileBackend virStorageFileBackendDir = {

 .storageFileAccess = virStorageFileBackendFileAccess,
 .storageFileChown = virStorageFileBackendFileChown,
-
-.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


diff --git a/src/storage_file/storage_file_backend_gluster.c 
b/src/storage_file/storage_file_backend_gluster.c
index 06ba99bfe3..252eb523af 100644
--- a/src/storage_file/storage_file_backend_gluster.c
+++ b/src/storage_file/storage_file_backend_gluster.c
@@ -255,75 +255,6 @@ virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
 return glfs_access(priv->vol, src->path, mode);
 }

-static int
-virStorageFileBackendGlusterReadlinkCallback(const char *path,
- char **linkpath,
- void *data)
-{
-virStorageFileBackendGlusterPrivPtr priv = data;
-size_t bufsiz = 0;
-ssize_t ret;
-struct stat st;
-g_autofree char *buf = NULL;
-
-*linkpath = NULL;
-
-if (glfs_stat(priv->vol, path, ) < 0) {
-virReportSystemError(errno,
- _("failed to stat gluster path '%s'"),
- path);
-return -1;
-}
-
-if (!S_ISLNK(st.st_mode))
-return 1;
-
- realloc:
-VIR_EXPAND_N(buf, bufsiz, 256);
-
-if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) {
-virReportSystemError(errno,
- _("failed to read link of gluster file '%s'"),
-  

[PATCH 23/39] storage: zfs: Don't split string if we need only first/last component

[Peter Krempa]

Use str(r)chr to find the correct bit rather than fully splitting the
string.

Signed-off-by: Peter Krempa 
---
 src/storage/storage_backend_zfs.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_backend_zfs.c 
b/src/storage/storage_backend_zfs.c
index e4ef06722f..cab1fd1637 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -102,7 +102,7 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool,
 virStorageVolDefPtr volume = NULL;
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 g_auto(GStrv) tokens = NULL;
-g_auto(GStrv) name_tokens = NULL;
+char *tmp;

 if (!(tokens = virStringSplitCount(volume_string, "\t", 0, )))
 return -1;
@@ -110,10 +110,9 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool,
 if (count != 3)
 goto cleanup;

-if (!(name_tokens = virStringSplitCount(tokens[0], "/", 0, )))
-goto cleanup;
-
-vol_name = name_tokens[count-1];
+vol_name = tokens[0];
+if ((tmp = strrchr(vol_name, '/')))
+vol_name = tmp + 1;

 if (vol == NULL)
 volume = virStorageVolDefFindByName(pool, vol_name);
@@ -218,7 +217,8 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool 
G_GNUC_UNUSED)
 g_autoptr(virCommand) cmd = NULL;
 g_auto(GStrv) lines = NULL;
 g_auto(GStrv) tokens = NULL;
-g_auto(GStrv) name_tokens = NULL;
+g_autofree char *name = g_strdup(def->source.name);
+char *tmp;

 /**
  * $ zpool get -Hp health,size,free,allocated test
@@ -230,13 +230,13 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool 
G_GNUC_UNUSED)
  *
  * Here we just provide a list of properties we want to see
  */
-if (!(name_tokens = g_strsplit(def->source.name, "/", 0)))
-goto cleanup;
+if ((tmp = strchr(name, '/')))
+*tmp = '\0';

 cmd = virCommandNewArgList(ZPOOL,
"get", "-Hp",
"health,size,free,allocated",
-   name_tokens[0],
+   name,
NULL);
 virCommandSetOutputBuffer(cmd, _props);
 if (virCommandRun(cmd, NULL) < 0)
-- 
2.29.2

[PATCH 29/39] virResctrlAllocGetUnused: Use g_autoptr for variables of virResctrlAlloc type

[Peter Krempa]

Refactor the handling of variables so that the cleanup section can be
sanitized.

Signed-off-by: Peter Krempa 
---
 src/util/virresctrl.c | 32 
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index c66cf4b087..53c202f99f 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -1874,8 +1874,8 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
 virResctrlAllocPtr
 virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
 {
-virResctrlAllocPtr ret = NULL;
-virResctrlAllocPtr alloc = NULL;
+g_autoptr(virResctrlAlloc) ret = NULL;
+g_autoptr(virResctrlAlloc) alloc_default = NULL;
 struct dirent *ent = NULL;
 g_autoptr(DIR) dirp = NULL;
 int rv = -1;
@@ -1890,17 +1890,18 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
 if (!ret)
 return NULL;

-alloc = virResctrlAllocGetDefault(resctrl);
-if (!alloc)
-goto error;
+alloc_default = virResctrlAllocGetDefault(resctrl);
+if (!alloc_default)
+return NULL;

-virResctrlAllocSubtract(ret, alloc);
-virObjectUnref(alloc);
+virResctrlAllocSubtract(ret, alloc_default);

 if (virDirOpen(, SYSFS_RESCTRL_PATH) < 0)
-goto error;
+return NULL;

 while ((rv = virDirRead(dirp, , SYSFS_RESCTRL_PATH)) > 0) {
+g_autoptr(virResctrlAlloc) alloc = NULL;
+
 if (STREQ(ent->d_name, "info"))
 continue;

@@ -1912,24 +1913,15 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not read schemata file for group %s"),
ent->d_name);
-goto error;
+return NULL;
 }

 virResctrlAllocSubtract(ret, alloc);
-virObjectUnref(alloc);
-alloc = NULL;
 }
 if (rv < 0)
-goto error;
-
- cleanup:
-virObjectUnref(alloc);
-return ret;
+return NULL;

- error:
-virObjectUnref(ret);
-ret = NULL;
-goto cleanup;
+return g_steal_pointer();
 }


-- 
2.29.2

[PATCH 07/39] Remove virStorageFileCanonicalizePath

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/libvirt_private.syms  |   1 -
 src/util/virstoragefile.c | 210 --
 src/util/virstoragefile.h |   8 --
 3 files changed, 219 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 62ccda467f..9208db2056 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3231,7 +3231,6 @@ virSocketAddrSetPort;


 # util/virstoragefile.h
-virStorageFileCanonicalizePath;
 virStorageFileGetNPIVKey;
 virStorageFileGetSCSIKey;
 virStorageFileParseBackingStoreStr;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 9df891b57c..e6bc723d1e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -214,213 +214,3 @@ virStorageFileParseBackingStoreStr(const char *str,
 *chainIndex = idx;
 return 0;
 }
-
-
-static char *
-virStorageFileCanonicalizeFormatPath(char **components,
- size_t ncomponents,
- bool beginSlash,
- bool beginDoubleSlash)
-{
-g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-size_t i;
-char *ret = NULL;
-
-if (beginSlash)
-virBufferAddLit(, "/");
-
-if (beginDoubleSlash)
-virBufferAddLit(, "/");
-
-for (i = 0; i < ncomponents; i++) {
-if (i != 0)
-virBufferAddLit(, "/");
-
-virBufferAdd(, components[i], -1);
-}
-
-/* if the output string is empty just return an empty string */
-if (!(ret = virBufferContentAndReset()))
-ret = g_strdup("");
-
-return ret;
-}
-
-
-static int
-virStorageFileCanonicalizeInjectSymlink(const char *path,
-size_t at,
-char ***components,
-size_t *ncomponents)
-{
-char **tmp = NULL;
-char **next;
-size_t ntmp = 0;
-int ret = -1;
-
-if (!(tmp = virStringSplitCount(path, "/", 0, )))
-goto cleanup;
-
-/* prepend */
-for (next = tmp; *next; next++) {
-if (VIR_INSERT_ELEMENT(*components, at, *ncomponents, *next) < 0)
-goto cleanup;
-
-at++;
-}
-
-ret = 0;
-
- cleanup:
-virStringListFreeCount(tmp, ntmp);
-return ret;
-}
-
-
-char *
-virStorageFileCanonicalizePath(const char *path,
-   virStorageFileSimplifyPathReadlinkCallback cb,
-   void *cbdata)
-{
-GHashTable *cycle = NULL;
-bool beginSlash = false;
-bool beginDoubleSlash = false;
-char **components = NULL;
-size_t ncomponents = 0;
-size_t i = 0;
-size_t j = 0;
-int rc;
-char *ret = NULL;
-g_autofree char *linkpath = NULL;
-g_autofree char *currentpath = NULL;
-
-if (path[0] == '/') {
-beginSlash = true;
-
-if (path[1] == '/' && path[2] != '/')
-beginDoubleSlash = true;
-}
-
-if (!(cycle = virHashNew(NULL)))
-goto cleanup;
-
-if (!(components = virStringSplitCount(path, "/", 0, )))
-goto cleanup;
-
-j = 0;
-while (j < ncomponents) {
-/* skip slashes */
-if (STREQ(components[j], "")) {
-VIR_FREE(components[j]);
-VIR_DELETE_ELEMENT(components, j, ncomponents);
-continue;
-}
-j++;
-}
-
-while (i < ncomponents) {
-/* skip '.'s unless it's the last one remaining */
-if (STREQ(components[i], ".") &&
-(beginSlash || ncomponents  > 1)) {
-VIR_FREE(components[i]);
-VIR_DELETE_ELEMENT(components, i, ncomponents);
-continue;
-}
-
-/* resolve changes to parent directory */
-if (STREQ(components[i], "..")) {
-if (!beginSlash &&
-(i == 0 || STREQ(components[i - 1], ".."))) {
-i++;
-continue;
-}
-
-VIR_FREE(components[i]);
-VIR_DELETE_ELEMENT(components, i, ncomponents);
-
-if (i != 0) {
-VIR_FREE(components[i - 1]);
-VIR_DELETE_ELEMENT(components, i - 1, ncomponents);
-i--;
-}
-
-continue;
-}
-
-/* check if the actual path isn't resulting into a symlink */
-if (!(currentpath = virStorageFileCanonicalizeFormatPath(components,
- i + 1,
- beginSlash,
- 
beginDoubleSlash)))
-goto cleanup;
-
-if ((rc = cb(currentpath, , cbdata)) < 0)
-goto cleanup;
-
-if (rc == 0) {
-if (virHashLookup(cycle, currentpath)) {
-virReportSystemError(ELOOP,
- _("Failed to canonicalize path '%s'"), 
path);
-   

[PATCH 02/39] virStorageSourceGetMetadata: Use depth limit instead of unique path checking

[Peter Krempa]

Prevent unbounded chains by limiting the recursion depth of
virStorageSourceGetMetadataRecurse to the maximum number of image layers
we limit anyways.

This removes the last use of virStorageSourceGetUniqueIdentifier which
will allow us to delete some crusty old infrastructure which isn't
really needed.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c|  4 ++-
 src/security/virt-aa-helper.c |  5 +++-
 src/storage_file/storage_source.c | 44 +--
 src/storage_file/storage_source.h |  1 +
 tests/virstoragetest.c|  3 ++-
 5 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f818fce271..cf6d41dcad 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7627,7 +7627,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,

 qemuDomainGetImageIds(cfg, vm, src, disksrc, , );

-if (virStorageSourceGetMetadata(src, uid, gid, report_broken) < 0)
+if (virStorageSourceGetMetadata(src, uid, gid,
+QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH,
+report_broken) < 0)
 return -1;

 for (n = src->backingStore; virStorageSourceIsBacking(n); n = 
n->backingStore) {
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c615305320..7e29e43c2e 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -937,9 +937,12 @@ get_files(vahControl * ctl)
 continue;
 /* XXX - if we knew the qemu user:group here we could send it in
  *so that the open could be re-tried as that user:group.
+ *
+ * The maximum depth is limited to 200 layers similarly to the qemu
+ * implementation.
  */
 if (!disk->src->backingStore)
-virStorageSourceGetMetadata(disk->src, -1, -1, false);
+virStorageSourceGetMetadata(disk->src, -1, -1, 200, false);

  /* XXX should handle open errors more careful than just ignoring them.
  */
diff --git a/src/storage_file/storage_source.c 
b/src/storage_file/storage_source.c
index ffe150a9b0..19b06b02b8 100644
--- a/src/storage_file/storage_source.c
+++ b/src/storage_file/storage_source.c
@@ -1297,11 +1297,9 @@ 
virStorageSourceGetMetadataRecurseReadHeader(virStorageSourcePtr src,
  uid_t uid,
  gid_t gid,
  char **buf,
- size_t *headerLen,
- GHashTable *cycle)
+ size_t *headerLen)
 {
 int ret = -1;
-const char *uniqueName;
 ssize_t len;

 if (virStorageSourceInitAs(src, uid, gid) < 0)
@@ -1312,19 +1310,6 @@ 
virStorageSourceGetMetadataRecurseReadHeader(virStorageSourcePtr src,
 goto cleanup;
 }

-if (!(uniqueName = virStorageSourceGetUniqueIdentifier(src)))
-goto cleanup;
-
-if (virHashHasEntry(cycle, uniqueName)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("backing store for %s (%s) is self-referential"),
-   NULLSTR(src->path), uniqueName);
-goto cleanup;
-}
-
-if (virHashAddEntry(cycle, uniqueName, NULL) < 0)
-goto cleanup;
-
 if ((len = virStorageSourceRead(src, 0, VIR_STORAGE_MAX_HEADER, buf)) < 0)
 goto cleanup;

@@ -1343,7 +1328,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr 
src,
virStorageSourcePtr parent,
uid_t uid, gid_t gid,
bool report_broken,
-   GHashTable *cycle,
+   size_t max_depth,
unsigned int depth)
 {
 virStorageFileFormat orig_format = src->format;
@@ -1352,9 +1337,16 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr 
src,
 g_autofree char *buf = NULL;
 g_autoptr(virStorageSource) backingStore = NULL;

-VIR_DEBUG("path=%s format=%d uid=%u gid=%u",
+VIR_DEBUG("path=%s format=%d uid=%u gid=%u depth=%u",
   NULLSTR(src->path), src->format,
-  (unsigned int)uid, (unsigned int)gid);
+  (unsigned int)uid, (unsigned int)gid, depth);
+
+if (depth > max_depth) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("backing store for %s is self-referential or too 
deeply nested"),
+   NULLSTR(src->path));
+return -1;
+}

 if (src->format == VIR_STORAGE_FILE_AUTO_SAFE)
 src->format = VIR_STORAGE_FILE_AUTO;
@@ -1369,7 +1361,7 @@ virStorageSourceGetMetadataRecurse(virStorageSourcePtr 
src,
 }

 if (virStorageSourceGetMetadataRecurseReadHeader(src, parent, uid, gid,
-  

[PATCH 18/39] bhyveParsePCIFbuf: Use g_strsplit instead of virStringSplitCount

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/bhyve/bhyve_parse_command.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 70f5ac42a0..d86d37b697 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -558,10 +558,8 @@ bhyveParsePCIFbuf(virDomainDefPtr def,

 virDomainVideoDefPtr video = NULL;
 virDomainGraphicsDefPtr graphics = NULL;
-char **params = NULL;
-char *param = NULL, *separator = NULL;
-size_t nparams = 0;
-size_t i = 0;
+g_auto(GStrv) **params = NULL;
+GStrv next;

 if (!(video = virDomainVideoDefNew(xmlopt)))
 goto cleanup;
@@ -579,11 +577,11 @@ bhyveParsePCIFbuf(virDomainDefPtr def,
 if (!config)
 goto error;

-if (!(params = virStringSplitCount(config, ",", 0, )))
+if (!(params = g_strsplit(config, ",", 0)))
 goto error;

-for (i = 0; i < nparams; i++) {
-param = params[i];
+for (next = params; *next; next++) {
+char *param = *next;
 if (!video->driver)
 video->driver = g_new0(virDomainVideoDriverDef, 1);

@@ -649,13 +647,11 @@ bhyveParsePCIFbuf(virDomainDefPtr def,
 if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0)
 goto error;

-g_strfreev(params);
 return 0;

  error:
 virDomainVideoDefFree(video);
 virDomainGraphicsDefFree(graphics);
-g_strfreev(params);
 return -1;
 }

-- 
2.29.2

[PATCH 19/39] virHostValidateGetCPUFlags: Use g_strsplit instead of virStringSplitCount

[Peter Krempa]

We don't need the count of elements to iterate the list.

Signed-off-by: Peter Krempa 
---
 tools/virt-host-validate-common.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/virt-host-validate-common.c 
b/tools/virt-host-validate-common.c
index fc43b2ddc8..7e9f5a667c 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -201,9 +201,8 @@ virBitmapPtr virHostValidateGetCPUFlags(void)
 do {
 char line[1024];
 char *start;
-char **tokens;
-size_t ntokens;
-size_t i;
+g_auto(GStrv) tokens = NULL;
+GStrv next;

 if (!fgets(line, sizeof(line), fp))
 break;
@@ -228,20 +227,18 @@ virBitmapPtr virHostValidateGetCPUFlags(void)

 /* Split the line using " " as a delimiter. The first token
  * will always be ":", but that's okay */
-if (!(tokens = virStringSplitCount(start, " ", 0, )))
+if (!(tokens = g_strsplit(start, " ", 0)))
 continue;

 /* Go through all flags and check whether one of those we
  * might want to check for later on is present; if that's
  * the case, set the relevant bit in the bitmap */
-for (i = 0; i < ntokens; i++) {
+for (next = tokens; *next; next++) {
 int value;

-if ((value = virHostValidateCPUFlagTypeFromString(tokens[i])) >= 0)
+if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0)
 ignore_value(virBitmapSetBit(flags, value));
 }
-
-virStringListFreeCount(tokens, ntokens);
 } while (1);

 VIR_FORCE_FCLOSE(fp);
-- 
2.29.2

[PATCH 01/39] qemuDomainStorageSourceValidateDepth: Define chain depth as macro

[Peter Krempa]

The magic constant will be used in one more place.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 76e8903dbc..f818fce271 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7471,6 +7471,7 @@ qemuDomainStorageAlias(const char *device, int depth)
  *
  * Returns 0 on success and -1 if the chain is too deep. Error is reported.
  */
+#define QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH 200
 int
 qemuDomainStorageSourceValidateDepth(virStorageSourcePtr src,
  int add,
@@ -7484,7 +7485,7 @@ qemuDomainStorageSourceValidateDepth(virStorageSourcePtr 
src,

 nlayers += add;

-if (nlayers > 200) {
+if (nlayers > QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH) {
 if (diskdst)
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("backing chains more than 200 layers deep are not 
"
-- 
2.29.2

[PATCH 16/39] virLogParseOutput: Replace virStringSplitCount by g_strsplit

[Peter Krempa]

Unfortunately here we do need the count of elements. Use g_strv_length
to calculate it so that virStringSplitCount can be removed later.

Signed-off-by: Peter Krempa 
---
 src/util/virlog.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 8a4cfbfdc2..3fb27fcb25 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1470,7 +1470,8 @@ virLogParseOutput(const char *src)
 /* split our format prio:destination:additional_data to tokens and parse
  * them individually
  */
-if (!(tokens = virStringSplitCount(src, ":", 0, )) || count < 2) {
+if (!(tokens = g_strsplit(src, ":", 0)) ||
+(count = g_strv_length(tokens))< 2) {
 virReportError(VIR_ERR_INVALID_ARG,
_("Malformed format for output '%s'"), src);
 return NULL;
-- 
2.29.2

[PATCH 21/39] virshParseRateStr: Use g_strsplit instead of virStringSplitCount

[Peter Krempa]

Count the elements after splitting the string.

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

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 37be298809..532169d8d7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -860,10 +860,10 @@ virshParseRateStr(vshControl *ctl,
 g_auto(GStrv) tok = NULL;
 size_t ntok;

-if (!(tok = virStringSplitCount(rateStr, ",", 0, )))
+if (!(tok = g_strsplit(rateStr, ",", 0)))
 return -1;

-if (ntok > 4) {
+if ((ntok = g_strv_length(tok)) > 4) {
 vshError(ctl, _("Rate string '%s' has too many fields"), rateStr);
 return -1;
 }
-- 
2.29.2

[PATCH 13/39] virLogParseOutputs: Refactor string list handling

[Peter Krempa]

Rewrite the code to remove the need to calculate the string list count.

Signed-off-by: Peter Krempa 
---
 src/util/virlog.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 004792db01..56059f62f1 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1612,22 +1612,22 @@ virLogParseOutputs(const char *src, virLogOutputPtr 
**outputs)
 {
 int at = -1;
 size_t noutputs = 0;
-size_t i, count;
 g_auto(GStrv) strings = NULL;
+GStrv next;
 virLogOutputPtr output = NULL;
 virLogOutputPtr *list = NULL;

 VIR_DEBUG("outputs=%s", src);

-if (!(strings = virStringSplitCount(src, " ", 0, )))
+if (!(strings = g_strsplit(src, " ", 0)))
 return -1;

-for (i = 0; i < count; i++) {
+for (next = strings; *next; next++) {
 /* virStringSplit may return empty strings */
-if (STREQ(strings[i], ""))
+if (STREQ(*next, ""))
 continue;

-if (!(output = virLogParseOutput(strings[i])))
+if (!(output = virLogParseOutput(*next)))
 return -1;

 /* let's check if a duplicate output does not already exist in which
-- 
2.29.2

[PATCH 08/39] virDomainDiskAddISCSIPoolSourceHost: use g_strsplit instead of virStringSplitCount

[Peter Krempa]

Count the elements directly using g_strv_length.

Signed-off-by: Peter Krempa 
---
 src/conf/domain_conf.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d050a519c6..2ab476dfdb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -32080,7 +32080,6 @@ virDomainDiskAddISCSIPoolSourceHost(virStorageSourcePtr 
src,
 virStoragePoolDefPtr pooldef)
 {
 g_auto(GStrv) tokens = NULL;
-size_t ntokens;

 /* Only support one host */
 if (pooldef->source.nhost != 1) {
@@ -32101,10 +32100,10 @@ 
virDomainDiskAddISCSIPoolSourceHost(virStorageSourcePtr src,
 src->hosts[0].port = 3260;

 /* iscsi volume has name like "unit:0:0:1" */
-if (!(tokens = virStringSplitCount(src->srcpool->volume, ":", 0, 
)))
+if (!(tokens = g_strsplit(src->srcpool->volume, ":", 0)))
 return -1;

-if (ntokens != 4) {
+if (g_strv_length(tokens) != 4) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected iscsi volume name '%s'"),
src->srcpool->volume);
-- 
2.29.2

[PATCH 10/39] virStorageFileParseBackingStoreStr: use g_strsplit instead of virStringSplitCount

[Peter Krempa]

The presence of the second element can be checked by looking at it
directly.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index e6bc723d1e..e3ec64486e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -192,17 +192,16 @@ virStorageFileParseBackingStoreStr(const char *str,
char **target,
unsigned int *chainIndex)
 {
-size_t nstrings;
 unsigned int idx = 0;
 char *suffix;
 g_auto(GStrv) strings = NULL;

 *chainIndex = 0;

-if (!(strings = virStringSplitCount(str, "[", 2, )))
+if (!(strings = g_strsplit(str, "[", 2)))
 return -1;

-if (nstrings == 2) {
+if (strings[0] && strings[1]) {
 if (virStrToLong_uip(strings[1], , 10, ) < 0 ||
 STRNEQ(suffix, "]"))
 return -1;
-- 
2.29.2

[PATCH 09/39] virJSONValueObjectDeflattenWorker: use g_strsplit instead of virStringSplitCount

[Peter Krempa]

The presence of the second element can be checked by looking at it
directly.

Signed-off-by: Peter Krempa 
---
 src/util/virjson.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index 82081db870..f376490288 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -2029,7 +2029,6 @@ virJSONValueObjectDeflattenWorker(const char *key,
 g_autoptr(virJSONValue) newval = NULL;
 virJSONValuePtr existobj;
 g_auto(GStrv) tokens = NULL;
-size_t ntokens = 0;

 /* non-nested keys only need to be copied */
 if (!strchr(key, '.')) {
@@ -2054,10 +2053,10 @@ virJSONValueObjectDeflattenWorker(const char *key,
 return 0;
 }

-if (!(tokens = virStringSplitCount(key, ".", 2, )))
+if (!(tokens = g_strsplit(key, ".", 2)))
 return -1;

-if (ntokens != 2) {
+if (!tokens[0] || !tokens[1]) {
 virReportError(VIR_ERR_INVALID_ARG,
_("invalid nested value key '%s'"), key);
 return -1;
-- 
2.29.2

[PATCH 06/39] tests: Remove testing of virStorageFileCanonicalizePath

[Peter Krempa]

Remove the last code using the function.

Signed-off-by: Peter Krempa 
---
 tests/virstoragetest.c | 100 -
 1 file changed, 100 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 157d577c7d..23cc72d85c 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -398,62 +398,6 @@ testStorageLookup(const void *args)
 }


-struct testPathCanonicalizeData
-{
-const char *path;
-const char *expect;
-};
-
-static const char *testPathCanonicalizeSymlinks[][2] =
-{
-{"/path/blah", "/other/path/huzah"},
-{"/path/to/relative/symlink", "../../actual/file"},
-{"/cycle", "/cycle"},
-{"/cycle2/link", "./link"},
-};
-
-static int
-testPathCanonicalizeReadlink(const char *path,
- char **linkpath,
- void *data G_GNUC_UNUSED)
-{
-size_t i;
-
-*linkpath = NULL;
-
-for (i = 0; i < G_N_ELEMENTS(testPathCanonicalizeSymlinks); i++) {
-if (STREQ(path, testPathCanonicalizeSymlinks[i][0])) {
-*linkpath = g_strdup(testPathCanonicalizeSymlinks[i][1]);
-
-return 0;
-}
-}
-
-return 1;
-}
-
-
-static int
-testPathCanonicalize(const void *args)
-{
-const struct testPathCanonicalizeData *data = args;
-g_autofree char *canon = NULL;
-
-canon = virStorageFileCanonicalizePath(data->path,
-   testPathCanonicalizeReadlink,
-   NULL);
-
-if (STRNEQ_NULLABLE(data->expect, canon)) {
-fprintf(stderr,
-"path canonicalization of '%s' failed: expected '%s' got 
'%s'\n",
-data->path, NULLSTR(data->expect), NULLSTR(canon));
-
-return -1;
-}
-
-return 0;
-}
-
 static virStorageSource backingchain[12];

 static void
@@ -602,7 +546,6 @@ mymain(void)
 int ret;
 struct testChainData data;
 struct testLookupData data2;
-struct testPathCanonicalizeData data3;
 struct testPathRelativeBacking data4;
 struct testBackingParseData data5;
 virStorageSourcePtr chain2; /* short for chain->backingStore */
@@ -1086,49 +1029,6 @@ mymain(void)
 TEST_LOOKUP_TARGET(80, "vda", chain3, "vda[2]", 0, NULL, NULL, NULL);
 TEST_LOOKUP_TARGET(81, "vda", NULL, "vda[3]", 0, NULL, NULL, NULL);

-#define TEST_PATH_CANONICALIZE(id, PATH, EXPECT) \
-do { \
-data3.path = PATH; \
-data3.expect = EXPECT; \
-if (virTestRun("Path canonicalize " #id, \
-   testPathCanonicalize, ) < 0) \
-ret = -1; \
-} while (0)
-
-TEST_PATH_CANONICALIZE(1, "/", "/");
-TEST_PATH_CANONICALIZE(2, "/path", "/path");
-TEST_PATH_CANONICALIZE(3, "/path/to/blah", "/path/to/blah");
-TEST_PATH_CANONICALIZE(4, "/path/", "/path");
-TEST_PATH_CANONICALIZE(5, "///", "/");
-TEST_PATH_CANONICALIZE(6, "//", "//");
-TEST_PATH_CANONICALIZE(7, "", "");
-TEST_PATH_CANONICALIZE(8, ".", ".");
-TEST_PATH_CANONICALIZE(9, "../", "..");
-TEST_PATH_CANONICALIZE(10, "../../", "../..");
-TEST_PATH_CANONICALIZE(11, "../../blah", "../../blah");
-TEST_PATH_CANONICALIZE(12, "/./././blah", "/blah");
-TEST_PATH_CANONICALIZE(13, ".././../././../blah", "../../../blah");
-TEST_PATH_CANONICALIZE(14, "/././", "/");
-TEST_PATH_CANONICALIZE(15, "./././", ".");
-TEST_PATH_CANONICALIZE(16, "blah/../foo", "foo");
-TEST_PATH_CANONICALIZE(17, "foo/bar/../blah", "foo/blah");
-TEST_PATH_CANONICALIZE(18, "foo/bar/.././blah", "foo/blah");
-TEST_PATH_CANONICALIZE(19, "/path/to/foo/bar/../../../../../../../../baz", 
"/baz");
-TEST_PATH_CANONICALIZE(20, "path/to/foo/bar/../../../../../../../../baz", 
"../../../../baz");
-TEST_PATH_CANONICALIZE(21, "path/to/foo/bar", "path/to/foo/bar");
-TEST_PATH_CANONICALIZE(22, "//foo//bar", "//foo/bar");
-TEST_PATH_CANONICALIZE(23, "/bar//foo", "/bar/foo");
-TEST_PATH_CANONICALIZE(24, "//../blah", "//blah");
-
-/* test paths with symlinks */
-TEST_PATH_CANONICALIZE(25, "/path/blah", "/other/path/huzah");
-TEST_PATH_CANONICALIZE(26, "/path/to/relative/symlink", 
"/path/actual/file");
-TEST_PATH_CANONICALIZE(27, "/path/to/relative/symlink/blah", 
"/path/actual/file/blah");
-TEST_PATH_CANONICALIZE(28, "/path/blah/yippee", 
"/other/path/huzah/yippee");
-TEST_PATH_CANONICALIZE(29, "/cycle", NULL);
-TEST_PATH_CANONICALIZE(30, "/cycle2/link", NULL);
-TEST_PATH_CANONICALIZE(31, "///", "/");
-
 #define TEST_RELATIVE_BACKING(id, TOP, BASE, EXPECT) \
 do { \
 data4.top =  \
-- 
2.29.2

[PATCH 04/39] storage_file: Remove virStorageFileBackendFsPriv

[Peter Krempa]

The private data structure is no longer used.

Signed-off-by: Peter Krempa 
---
 src/storage_file/storage_file_backend_fs.c | 20 
 1 file changed, 20 deletions(-)

diff --git a/src/storage_file/storage_file_backend_fs.c 
b/src/storage_file/storage_file_backend_fs.c
index f34ffd5fc8..8422543057 100644
--- a/src/storage_file/storage_file_backend_fs.c
+++ b/src/storage_file/storage_file_backend_fs.c
@@ -40,27 +40,12 @@
 VIR_LOG_INIT("storage.storage_backend_fs");


-typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv;
-typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr;
-
-struct _virStorageFileBackendFsPriv {
-char *canonpath; /* unique file identifier (canonical path) */
-};
-
-
 static void
 virStorageFileBackendFileDeinit(virStorageSourcePtr src)
 {
-virStorageDriverDataPtr drv = src->drv;
-virStorageFileBackendFsPrivPtr priv = drv->priv;
-
 VIR_DEBUG("deinitializing FS storage file %p (%s:%s)", src,
   virStorageTypeToString(virStorageSourceGetActualType(src)),
   src->path);
-
-
-VIR_FREE(priv->canonpath);
-VIR_FREE(priv);
 }


@@ -68,17 +53,12 @@ static int
 virStorageFileBackendFileInit(virStorageSourcePtr src)
 {
 virStorageDriverDataPtr drv = src->drv;
-virStorageFileBackendFsPrivPtr priv = NULL;

 VIR_DEBUG("initializing FS storage file %p (%s:%s)[%u:%u]", src,
   virStorageTypeToString(virStorageSourceGetActualType(src)),
   src->path,
   (unsigned int)drv->uid, (unsigned int)drv->gid);

-priv = g_new0(virStorageFileBackendFsPriv, 1);
-
-drv->priv = priv;
-
 return 0;
 }

-- 
2.29.2

[PATCH 00/39] String cleaning

[Peter Krempa]

It's spring, let's clean up some string cruft.

Peter Krempa (39):
  qemuDomainStorageSourceValidateDepth: Define chain depth as macro
  virStorageSourceGetMetadata: Use depth limit instead of unique path
checking
  Remove virStorageSourceGetUniqueIdentifier file backend API
  storage_file: Remove virStorageFileBackendFsPriv
  virStorageFileBackendGlusterPriv: Remove 'canonpaht'
  tests: Remove testing of virStorageFileCanonicalizePath
  Remove virStorageFileCanonicalizePath
  virDomainDiskAddISCSIPoolSourceHost: use g_strsplit instead of
virStringSplitCount
  virJSONValueObjectDeflattenWorker: use g_strsplit instead of
virStringSplitCount
  virStorageFileParseBackingStoreStr: use g_strsplit instead of
virStringSplitCount
  util: virlog: Use g_auto(GStrv) instead of g_strfreev in cleanup
section
  util: virlog: Remove pointless 'cleanup' labels
  virLogParseOutputs: Refactor string list handling
  virLogParseFilters: Refactor string list handling
  virLogParseFilter: Replace virStringSplitCount by g_strsplit
  virLogParseOutput: Replace virStringSplitCount by g_strsplit
  virshParseEventStr: Use g_strsplit and automatic memory freeing
  bhyveParsePCIFbuf: Use g_strsplit instead of virStringSplitCount
  virHostValidateGetCPUFlags: Use g_strsplit instead of
virStringSplitCount
  virshParseRateStr: Refactor cleanup
  virshParseRateStr: Use g_strsplit instead of virStringSplitCount
  virStorageSourceParseBackingJSONUriCookies: Use g_strsplit instead of
virStringSplitCount
  storage: zfs: Don't split string if we need only first/last component
  virStorageBackendZFSRefreshPool: Reduce scope of 'tokens'
  storage: zfs: Use g_strsplit instead of virStringSplitCount
  virResctrlGetCacheInfo: Restrict variable scope and use automatic
freeing
  virResctrlAllocNewFromInfo: Restrict variable scope and use automatic
freeing
  virResctrlAllocNewFromInfo: Use g_autoptr for 'ret'
  virResctrlAllocGetUnused: Use g_autoptr for variables of
virResctrlAlloc type
  util: virresctrl: Use automatic memory freeing
  util: virresctrl: Remove empty 'cleanup' sections
  util: virresctrl: Use g_strsplit instead of virStringSplitCount
  xenParsePCI: Replace virStringSplitCount by g_strsplit
  xenParseXLVnuma: Replace virStringSplitCount by g_strsplit
  openvzParseBarrierLimit: Rework string handling
  virSystemdActivationInitFromNames: Replace virStringSplit by
g_strsplit
  virVMXParseConfig: Replace virStringSplitCount by g_strsplit
  util: virstring: Remove the virStringSplitCount wrapper funcion
  tests: string: Remove pointless test for virStringListFreeCount

 src/bhyve/bhyve_parse_command.c   |  14 +-
 src/conf/domain_conf.c|   5 +-
 src/libvirt_private.syms  |   3 -
 src/libxl/xen_common.c|   5 +-
 src/libxl/xen_xl.c|   4 +-
 src/openvz/openvz_conf.c  |  21 +-
 src/qemu/qemu_domain.c|   7 +-
 src/security/virt-aa-helper.c |   5 +-
 src/storage/storage_backend_zfs.c |  31 +-
 src/storage_file/storage_file_backend.h   |   4 -
 src/storage_file/storage_file_backend_fs.c|  44 ---
 .../storage_file_backend_gluster.c|  73 -
 src/storage_file/storage_source.c |  81 ++---
 src/storage_file/storage_source.h |   1 +
 .../storage_source_backingstore.c |   9 +-
 src/util/virjson.c|   5 +-
 src/util/virlog.c |  86 +++---
 src/util/virresctrl.c | 279 +++---
 src/util/virstoragefile.c | 215 +-
 src/util/virstoragefile.h |   8 -
 src/util/virstring.c  |  20 --
 src/util/virstring.h  |   6 -
 src/util/virsystemd.c |   9 +-
 src/vmx/vmx.c |   7 +-
 tests/virstoragetest.c| 103 +--
 tests/virstringtest.c |  23 --
 tools/virsh-domain.c  |  39 +--
 tools/virt-host-validate-common.c |  13 +-
 28 files changed, 237 insertions(+), 883 deletions(-)

-- 
2.29.2

Re: [libvirt PATCH 2/2] ci: Call meson consistently

[Andrea Bolognani]

On Wed, 2021-03-31 at 23:31 +0200, Martin Kletzander wrote:
> On Fri, Mar 26, 2021 at 11:35:02AM +0100, Andrea Bolognani wrote:
> > We should always pass --werror and display the contents of the
> > log file in case of failure.
> 
> Any reason why the lines are not in one place?  What I did in libnbd
> (first draft, still up for review) is that I just took all the lines of
> the script and put them inside `ci/build_script.sh` with only the most
> basic conditionals to accommodate various types of runs.  That way
> common things are in one place.  It could take a parameter (like what
> ninja target to run) and you can run things after that (like `website`
> and `potfiles` jobs do.  Just an idea.

As is often the case, the raisins are mostly hysterical ;)

I think the initial idea was to keep things as simple as possible,
but I agree with you that when you have seven (!) build recipes, most
of which are almost identical, it makes sense to think about
consolidating them to a single location.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] test: Update screenshot

[Andrea Bolognani]

On Thu, 2021-04-01 at 13:39 +0200, Michal Privoznik wrote:
> While having screenshot of NeXT Cube running WorldWideWeb, I
> think this deserves a little upgrade. Something more fancy.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/test/test-screenshot.png | Bin 33443 -> 1855 bytes
>  1 file changed, 0 insertions(+), 0 deletions(-)

This whole git commit's what I'm thinking of...

You wouldn't get a

  Reviewed-by: Andrea Bolognani 

from any other guy :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH RFC 0/3] Add checkpoint/restore support to LXC using CRIU

[Julio Faracco]

Hi Michal and Martin,

Thanks for your reply.
Just an explanation. I'm not interested directly in developing this
specific feature.
If there is a GSoC student addressed to this... Excellent.
I'm interested in developing snapshot and container migration which
unfortunately requires this feature.
Unless you have another opinion.

--
Julio Faracco

Em qui., 1 de abr. de 2021 às 07:33, Michal Privoznik
 escreveu:
>
> On 4/1/21 12:01 AM, Martin Kletzander wrote:
> > On Sat, Feb 27, 2021 at 01:14:29AM -0300, Julio Faracco wrote:
> >> Hi guys,
> >>
> >
> > Hi and sorry for not replying earlier.
> >
>
> Yeah, sorry. I have this marked for review and yet still haven't done so.
>
> >> I marked this series as RFC to discuss some points. I'm interested in
> >> enhancing this specific part of LXC. So, some questions that I would
> >> like to hear as a feedback from community:
> >> 1. I decided to use a tar to compress all CRIU img files into a single
> >> file. Any other suggestions?
> >> 2. If no is the answer to question above, is there a consensus on
> >> preferring to use command line calls or libraries? I would like to use
> >> libtar for instance. I personally think that this approach is ugly.
> >> Not sure if I'm able to do that. The same for CRIU.
> >
> > I remember that for CRIU, back when we were trying to do that, the issue
> > was that the commands were not atomic, did not properly report error
> > messages and maybe something more along the lines.  Either there was no
> > library interface or it was not MT-safe, basically there were couple of
> > issues like that which we were not able to deal with.
> >
> > I do not really remember all the details.  Maybe Michal does as I think
> > he suggested the idea back then.  I Cc'd him.  In the worst scenario we
> > will need to figure this all out again ;)
>
> IIRC the main problem was that we wanted CRIU to be able to send its
> data over a TCP connection. Back then, when a GSoC student was looking
> at this, CRIU was only able to store data into a file (or even multiple
> files in a directory?) and wasn't able to create server/client
> connection. Maybe this has changed since then? If not, then we can use
> tar, sure. And to transfer data we can use so called tunnelled
> migration, where the migration stream is sent over libvirt connection
> rather than directly to the other side (because then we would have to
> have nc or similar involved).
>
> https://libvirt.org/migration.html#transporttunnel
>
> Another issue was that it couldn't handle all namespaces (but I'm not
> certain - it was 5 years ago).
>
> But let me find some time and review patches.
>
> Michal
>

Re: [libvirt PATCHv2] docs: secret: do not use a valuable password

[Erik Skultety]

On Thu, Apr 01, 2021 at 02:54:54PM +0200, Ján Tomko wrote:
> Our website displays " **" as the example password.
> Despite its tradition rooted in 18th Century French literature,
> we cannot assume it to be public.
> 
> Use *** as suggested by Erik instead.
> 
> Signed-off-by: Ján Tomko 
> ---
ACK ;)

Erik

[libvirt PATCHv2] docs: secret: do not use a valuable password

[Ján Tomko]

Our website displays " **" as the example password.
Despite its tradition rooted in 18th Century French literature,
we cannot assume it to be public.

Use *** as suggested by Erik instead.

Signed-off-by: Ján Tomko 
---
 docs/formatsecret.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 9dc9cdf288..c0f16f700d 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -405,7 +405,7 @@ Secret value set
 
 
 
-# MYSECRET=`printf %s "open sesame" | base64`
+# MYSECRET=`printf %s "***" | base64`
 # virsh secret-set-value 6dd3e4a5-1d76-44ce-961f-f119f5aad935 $MYSECRET
 Secret value set
 
-- 
2.29.2

Re: [libvirt PATCH 5/4] meson: Don't check whether /usr/local/bin/grep is GNU grep

[Erik Skultety]

On Thu, Apr 01, 2021 at 02:17:14PM +0200, Andrea Bolognani wrote:
> Since /usr/local is where ports live, it's reasonable to assume
> that a grep binary found in there will have been installed via
> ports and will thus be GNU grep.
> 
> Suggested-by: Erik Skultety 
> Signed-off-by: Andrea Bolognani 
> ---

Feels weird ACKing this, but FWIW:
Reviewed-by: Erik Skultety 

Re: [libvirt PATCH] docs: secret: do not use a valuable password

[Erik Skultety]

On Thu, Apr 01, 2021 at 02:02:01PM +0200, Ján Tomko wrote:
> Our website displays " **" as the example password.
> Despite its tradition rooted in 18th century French literature,
> we cannot assume it to be public.
> 
> Use a random string of letters and numbers instead.
> 
> Signed-off-by: Ján Tomko 
> ---
>  docs/formatsecret.html.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
> index 9dc9cdf288..19613cb5dc 100644
> --- a/docs/formatsecret.html.in
> +++ b/docs/formatsecret.html.in
> @@ -405,7 +405,7 @@ Secret value set
>  
>  
>  
> -# MYSECRET=`printf %s "open sesame" | base64`
> +# MYSECRET=`printf %s "dQw4w9WgXcQ" | base64`

NACK, hunter2 is preferrable.

Regards,
Erik

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Andrea Bolognani]

On Thu, 2021-04-01 at 12:03 +0200, Erik Skultety wrote:
> On Thu, Apr 01, 2021 at 11:51:15AM +0200, Andrea Bolognani wrote:
> > I disagree that the hunk should be squashed in, though, since this
> > patch simply changes the error message and not what's been checked.
> > It should go in separately, either after this series or before it.
> > 
> > Do you want me to post it in that form, or would you rather do that
> > yourself since you have already done the work? Whatever is more
> > convenient for you :)
> 
> I honestly don't care, so if you squash it or append another patch to your
> series before merging - up to you.

Okay, posted as a separate patch here:

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

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH 5/4] meson: Don't check whether /usr/local/bin/grep is GNU grep

[Andrea Bolognani]

Since /usr/local is where ports live, it's reasonable to assume
that a grep binary found in there will have been installed via
ports and will thus be GNU grep.

Suggested-by: Erik Skultety 
Signed-off-by: Andrea Bolognani 
---
 build-aux/meson.build | 4 
 1 file changed, 4 deletions(-)

diff --git a/build-aux/meson.build b/build-aux/meson.build
index 1095982397..e491bdeebc 100644
--- a/build-aux/meson.build
+++ b/build-aux/meson.build
@@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd'
 if not grep_prog.found()
   error('GNU grep not found')
 endif
-grep_cmd = run_command(grep_prog, '--version')
-if grep_cmd.stdout().startswith('grep (BSD grep')
-  error('GNU grep not found')
-endif
   endif
 elif host_machine.system() == 'darwin'
   grep_prog = find_program('ggrep')
-- 
2.26.3

[libvirt PATCH] docs: secret: do not use a valuable password

[Ján Tomko]

Our website displays " **" as the example password.
Despite its tradition rooted in 18th century French literature,
we cannot assume it to be public.

Use a random string of letters and numbers instead.

Signed-off-by: Ján Tomko 
---
 docs/formatsecret.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 9dc9cdf288..19613cb5dc 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -405,7 +405,7 @@ Secret value set
 
 
 
-# MYSECRET=`printf %s "open sesame" | base64`
+# MYSECRET=`printf %s "dQw4w9WgXcQ" | base64`
 # virsh secret-set-value 6dd3e4a5-1d76-44ce-961f-f119f5aad935 $MYSECRET
 Secret value set
 
-- 
2.29.2

[PATCH] test: Update screenshot

[Michal Privoznik]

While having screenshot of NeXT Cube running WorldWideWeb, I
think this deserves a little upgrade. Something more fancy.

Signed-off-by: Michal Privoznik 
---
 src/test/test-screenshot.png | Bin 33443 -> 1855 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)

diff --git a/src/test/test-screenshot.png b/src/test/test-screenshot.png
index 
3991eabb228631ec86382273a3ea998cb9adf5bf..1c0053236fb2eec1afe1d8165cb9c197cdb36168
 100644
GIT binary patch
literal 1855
zcmbVNeKeF=7$3ZNMbQ$wNvdV)gguzd}?4Hi?(U~ClRkAR_P%A
zRUBqvjbI}Ovy<0MVXK?QJ5Ccqw9^1OJaq|p0RaaV<28%29XG3v(YRC~Vq0%A#eFU(Z8dcUMp4w?-()vLah)$#
zxV`KjOLG7|zMPZXnue~5wvEibM#W?Rf+N}JFP{64R|OX{Wa31r>T__sdy2w?kIK$I
zp%!nwiQrxGf%H|k{^#Vf-7y+>YzN;cqcXGl3>usYa+vR?+g{PfmFyhnOBjPME
zD#8?%>=JZ2;pM38BabiXRl!gY#%W4eRY#SMt)h0tPVNl0tRx?v^kUDW{cZU?TA}lf
zY}6jIhcSKF7r`M=b%+R}!WKT9jh5xZzURZpy`6HAMU4fHl*fAkG?(b0(Oa}d>4=vs
zw~?nz8fJ```LY@OCyq4!2HNVC0M-Q@8)J3ex``4u2LV!JH$@QZW}YUQ4aYSA7d`JV=NqkJNm?;TJDl{BL7U
zOe}4VrionI+^&_Xo^5vkWxl2%`f}tuXxhl`wJ=5>2_Iz8%20?U*i&$tmJW(j7oyuk
zR9j$ULe+A?1zHyUw%AS>^W^oye#kzsCKtqg%lJ)i%^m6O>ehKds-(ja#_HSsI2{L|
zpuu^$C#2mFzzLr_-u9NI2|5pG1@#m@UxLWaigfhm^beLn`KF8PoDy|X*Gl%t?irq1m0_ls
zj3#16p6_BX_4?egQWk?Yr%(Y7gW}FnKLdh2QC5I3o3ktcMMiJ5<0QST*
z`_Fj65V5iA^5&_x?B?g(!E$)nABtr*~FHPQWC_A9rUZi^`n3cqRO?XWPkjtf;KNhz#;fPjy3-KV+zcYXjrC
zBq+6JKsrLC7O!ntZy44P^_KpaK1~<(yNXFvAExI&~kq
z+YK76soiC?aoP%g$hkS(&)Pj>KTMg_NgPP0+OdAVHU3+@1$#*{9hP
zq62*dk)$kF6$m1!55ekg`P`E>vZshzY((+~IYb~{p0OwwhK%v
VO3X8K*$*B@LeS0-f1z(I{yV?eC1L;o

literal 33443
zcmYJa1za1=7d9HaXmKqTT!TxoQrrs^x8UxsrMOFQD}^A%-Q68ZaEIdV&=v|e@Bh2s
zy_?@AJ7;!IcI?b^o;e%+NmULTgB$|@0AMS~OKSiC$Y1~f>BAei03hikJq7=vF_+g+
z0Ra3M0f68z0N^iN6nq2#cya*%Ctv{JeI@`v;*#60E(*^;G*gn32E0ZlPWHf4=q~bl
z?f?KL{(mO|Y$NCyE=2QCP?15~#zI5qVj!dMDu9c=c*y8_XqbA_xVSl6**aL#c=)M8EpF$Ws7XLR1&*J6c!swqP7lT&}pdc;z*=O~9jda80tJ$p{f;WGCxA;dFe`EAyoL0{#h(#`{h56{Fjm*`2Q$O!n~
zS-G(2UEUdZu}h^}V5alKATo9|EYarfM~pdBRIG3LtFj+lrZ=tM9=<+Zh)JDiKZsk`
zG}vh^==Z~>@HDo%XVcVdPjP|`fGroj@vS%FM3AG|^Gj@n(hgb%N1D<;?cB4q4=o
zquIqSA#^MFtvRL_J9FtfhBe5DsKITyCMJ2sMOk0D>5#Dip?iXfKRwX?p5zknN!_5p
z^YN4q`DuXWHlM;>5r-jBuLLi749N+a%H~=1gHrGNb!y8-qV`Q0V7JN`);wEW=^utKdrt^2lK?U)`=XZCOirL)K
zFrp*lY{gz7h9OApOBFLtY|#qgO-aF49I!GzM%^Ks9i5Fmx%Ll2jf^C-Q7BMF5qs;#
z9C7CeXif#eQs9yOrrm2;SVSfTOT$|5(kc7#`5Awg+ueqSdYxHred`1(bIY}~_#
z+r6KIiK-<>GuS>AMK@~bC*!Q53I`5U2*h}#Pc9@U$?dY`-K{LG^qUpzQH<>J}dqBDZF5jkBQt=VjcacvbM;!+oXREPO)=Q{?xi8>ROFlOP@n`OKB!mUdj
zVu?*?%cODxvQu{2TX;FRno&$K`(w`WUdu?wlex(7UvKlB)vfuX0!rh*qAFjpbPDt_mdOHd5!`7J?Z

Re: [PATCH] XML update in

[Ján Tomko]

On a Wednesday in 2021, Ján Tomko wrote:

On a Wednesday in 2021, Kristina Hanicova wrote:

Previously, we accepted empty bridge name, because some old versions of
VMWare Workstation did not put it into the config. But this doesn't make
much sense - to have an interface type bridge with no name. We
circumvented this problem by generating an empty name but that is
equally wrong.

Therefore, fill in missing bridge names (according to the documentation
[1] the default bridge name is VMnet0) and error out if bridge name is
missing.

This partially reverts f246cdb5aca13ac9409b2ad43087e3078615ffcb

1: 
https://docs.vmware.com/en/VMware-Workstation-Player-for-Linux/16.0/com.vmware.player.linux.using.doc/GUID-BAFA66C3-81F0-4FCA-84C4-D9F7D258A60A.html

Signed-off-by: Kristina Hanicova 
---
src/vmx/vmx.c  | 10 +++---
tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx |  2 ++
tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml |  4 ++--
tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.vmx |  1 +
tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml |  2 +-
tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.vmx |  2 ++
tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml |  4 ++--
tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.vmx |  1 +
tests/xml2vmxdata/xml2vmx-ws-in-the-wild-2.xml |  2 +-
9 files changed, 15 insertions(+), 13 deletions(-)



Reviewed-by: Ján Tomko 


Now pushed.



Jano


Dtto


signature.asc
Description: PGP signature

Re: Gsoc2021

[Ján Tomko]

On a Wednesday in 2021, Aaryan Singh wrote:

Hello,


This is Aaryan, a 2nd-year undergraduate student at  Kalinga institute of
industrial technology.


Welcome, Aaryan!

Jano


I have a keen interest in the " test driver API coverage " project. since I
have worked in cloud computing and DevOps that's why I am this interested
in this project and would love to share my idea and develop it.
Thank you


signature.asc
Description: PGP signature

Re: Gsoc2021

[Michal Privoznik]

On 3/31/21 3:01 PM, Aaryan Singh wrote:

Hello,


This is Aaryan, a 2nd-year undergraduate student at Kalinga institute of 
industrial technology.


I have a keen interest in the " test driver API coverage " project. 
since I have worked in cloud computing and DevOps that's why I am this 
interested in this project and would love to share my idea and develop it.

Thank you


Let's take this off the list. I've replied privately.

Michal

Re: [PATCH RFC 0/3] Add checkpoint/restore support to LXC using CRIU

[Michal Privoznik]

On 4/1/21 12:01 AM, Martin Kletzander wrote:

On Sat, Feb 27, 2021 at 01:14:29AM -0300, Julio Faracco wrote:

Hi guys,



Hi and sorry for not replying earlier.



Yeah, sorry. I have this marked for review and yet still haven't done so.


I marked this series as RFC to discuss some points. I'm interested in
enhancing this specific part of LXC. So, some questions that I would
like to hear as a feedback from community:
1. I decided to use a tar to compress all CRIU img files into a single
file. Any other suggestions?
2. If no is the answer to question above, is there a consensus on
preferring to use command line calls or libraries? I would like to use
libtar for instance. I personally think that this approach is ugly.
Not sure if I'm able to do that. The same for CRIU.


I remember that for CRIU, back when we were trying to do that, the issue
was that the commands were not atomic, did not properly report error
messages and maybe something more along the lines.  Either there was no
library interface or it was not MT-safe, basically there were couple of
issues like that which we were not able to deal with.

I do not really remember all the details.  Maybe Michal does as I think
he suggested the idea back then.  I Cc'd him.  In the worst scenario we
will need to figure this all out again ;)


IIRC the main problem was that we wanted CRIU to be able to send its 
data over a TCP connection. Back then, when a GSoC student was looking 
at this, CRIU was only able to store data into a file (or even multiple 
files in a directory?) and wasn't able to create server/client 
connection. Maybe this has changed since then? If not, then we can use 
tar, sure. And to transfer data we can use so called tunnelled 
migration, where the migration stream is sent over libvirt connection 
rather than directly to the other side (because then we would have to 
have nc or similar involved).


https://libvirt.org/migration.html#transporttunnel

Another issue was that it couldn't handle all namespaces (but I'm not 
certain - it was 5 years ago).


But let me find some time and review patches.

Michal

Release of libvirt-7.2.0

[Jiri Denemark]

The 7.2.0 release of both libvirt and libvirt-python is tagged and
signed tarballs and source RPMs are available at

https://libvirt.org/sources/
https://libvirt.org/sources/python/

Thanks everybody who helped with this release by sending patches,
reviewing, testing, or providing any other feedback. Your work is
greatly appreciated.

* New features

  * qemu: Implement domain memory dirty rate calculation API

New API ``virDomainStartDirtyRateCalc()`` and virsh command
``domdirtyrate-calc`` are added to start calculating a live domain's
memory dirty rate.

  * qemu: Support reporting memory dirty rate stats

The memory dirty rate stats can be obtained through ``virsh domstats
--dirtyrate`` via the virConnectGetAllDomainStats API.

  * qemu: Full disk backups via ``virDomainBackupBegin``

The qemu hypervisor driver now allows taking full disk backups via the
``virDomainBackupBegin`` API and the corresponding virsh wrapper.

In future releases the feature will be extended to also support incremental
backups (where only the difference since the last backup is copied) when
qemu adds the required functionality.

  * Add support for audio backend specific settings

With this release a new  element is introduced that allows
users to configure audio output for their guests.

* Improvements

  * qemu: Compatibility with QEMU 6.0 for certain hot-(un)-plug operations

Libvirt 7.2.0 is required for compatibility with the upcoming QEMU 6.0
release for hotplug and hotunplug of certain devices and helpers, such as
iothreads, chardevs, RNG devices, disks with secret, ...

  * qemu: Various improvements to embedded mode

Embedded mode for the QEMU driver, as well as the ``virt-qemu-run`` tool
saw improvements in handling of domain life cycle, temporary directories
creation (important when using disk secrets) and other minor fixes.

  * Documentation of split daemon related config files

Split daemons read configuration files upon their start. These were never
documented though.

* Bug fixes

  * Check host CPU for forbidden features

CPU feature policy did not work as expected with ``host-passthrough`` and
features supported by physical host. CPU features were not filtered out
when ``@check`` was set to ``full``.

  * Fix virNetworkUpdate() to work with split daemons

Due to a bug in our code, virNetworkUpdate() did not work with split daemon
unless management application connected to virtnetworkd directly.

  * qemu: increase locked memory limit when a vDPA device is present

Just like VFIO devices, vDPA devices may need to have all guest memory
pages locked/pinned in order to operate properly. These devices are now
included when calculating the limit for memory lock.

  * Don't log error if SRIOV PF has no associated netdev

Some SRIOV PFs don't have a netdev associated with them in which case
libvirtd reported an error and refused to start. This is now fixed.

  * qemu: Only raise memlock limit if necessary

Attempting to set the memlock limit might fail if we're running
in a containerized environment where ``CAP_SYS_RESOURCE`` is not
available, and if the limit is already high enough there's no
point in trying to raise it anyway.

  * Restore security context of swtpm.log

If a guest with emulated TPM was started and the daemon was restarted
afterwards, the security context of the per-domain ``swtpm.log`` file was
not restored on domain shutdown leaving it unable to be started again.

  * virtlogd|virtlockd: Fixed crash when upgrading the daemons in-place

A bug preventing the in-place upgrade of ``virtlogd`` and ``virtlockd``
daemons was fixed, so they can again be upgraded without dropping the log
file descriptors or locks on files.

Enjoy.

Jirka

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Erik Skultety]

On Thu, Apr 01, 2021 at 11:51:15AM +0200, Andrea Bolognani wrote:
> On Thu, 2021-04-01 at 11:23 +0200, Erik Skultety wrote:
> > Please squash this in before merging:
> > 
> > diff --git a/build-aux/meson.build b/build-aux/meson.build
> > index 1095982397..e491bdeebc 100644
> > --- a/build-aux/meson.build
> > +++ b/build-aux/meson.build
> > @@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd'
> >  if not grep_prog.found()
> >error('GNU grep not found')
> >  endif
> > -grep_cmd = run_command(grep_prog, '--version')
> > -if grep_cmd.stdout().startswith('grep (BSD grep')
> > -  error('GNU grep not found')
> > -endif
> 
> Mh, so what you're saying is that we can assume that
> /usr/local/bin/grep is *always* going to be GNU grep? That's probably
> a fair assumption.

I'd be surprised if a symlink existed in /usr/local/bin/grep pointing to the
distro's BSD grep, doesn't sound like FreeBSD hierarchy standards :).

> 
> I disagree that the hunk should be squashed in, though, since this
> patch simply changes the error message and not what's been checked.
> It should go in separately, either after this series or before it.
> 
> Do you want me to post it in that form, or would you rather do that
> yourself since you have already done the work? Whatever is more
> convenient for you :)

I honestly don't care, so if you squash it or append another patch to your
series before merging - up to you.

Erik

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Andrea Bolognani]

On Thu, 2021-04-01 at 11:23 +0200, Erik Skultety wrote:
> Please squash this in before merging:
> 
> diff --git a/build-aux/meson.build b/build-aux/meson.build
> index 1095982397..e491bdeebc 100644
> --- a/build-aux/meson.build
> +++ b/build-aux/meson.build
> @@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd'
>  if not grep_prog.found()
>error('GNU grep not found')
>  endif
> -grep_cmd = run_command(grep_prog, '--version')
> -if grep_cmd.stdout().startswith('grep (BSD grep')
> -  error('GNU grep not found')
> -endif

Mh, so what you're saying is that we can assume that
/usr/local/bin/grep is *always* going to be GNU grep? That's probably
a fair assumption.

I disagree that the hunk should be squashed in, though, since this
patch simply changes the error message and not what's been checked.
It should go in separately, either after this series or before it.

Do you want me to post it in that form, or would you rather do that
yourself since you have already done the work? Whatever is more
convenient for you :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] docs: formatnetworkport: Fix typos

[Erik Skultety]

On Thu, Apr 01, 2021 at 11:55:42AM +0800, Han Han wrote:
> Signed-off-by: Han Han 
> ---
Reviewed-by: Erik Skultety 

Re: [libvirt PATCH 0/4] meson: Make syntax-check work on FreeBSD and macOS

[Erik Skultety]

On Wed, Mar 24, 2021 at 07:37:56PM +0100, Andrea Bolognani wrote:
> Test pipeline:
> 
>   https://gitlab.com/abologna/libvirt/-/pipelines/275868429
> 
> Note that
> 
>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/101
> 
> needs to be merged and the data in ci/cirrus/ regenerated
> accordingly before this can be pushed without introducing CI
> failures.

Reviewed-by: Erik Skultety 

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Erik Skultety]

On Thu, Apr 01, 2021 at 11:10:05AM +0200, Erik Skultety wrote:
> On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
> > Currently, if GNU grep is not installed on a FreeBSD system the
> > configuration step will fail with
> > 
> >   Program grep found: YES (/usr/bin/grep)
> >   Program /usr/local/bin/grep found: NO
> > 
> >   ERROR: Program '/usr/local/bin/grep' not found
> > 
> > which is confusing and not very useful; after this change, the
> > message will be
> > 
> >   Program grep found: YES (/usr/bin/grep)
> >   Program /usr/local/bin/grep found: NO
> > 
> >   ERROR: Problem encountered: GNU grep not found
> > 
> > instead, which should do a better job helping the user figure
> > out that they need to install GNU grep from ports to proceed.
> > 
> > Signed-off-by: Andrea Bolognani 
> > ---
> Reviewed-by: Erik Skultety 

Please squash this in before merging:

diff --git a/build-aux/meson.build b/build-aux/meson.build
index 1095982397..e491bdeebc 100644
--- a/build-aux/meson.build
+++ b/build-aux/meson.build
@@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd'
 if not grep_prog.found()
   error('GNU grep not found')
 endif
-grep_cmd = run_command(grep_prog, '--version')
-if grep_cmd.stdout().startswith('grep (BSD grep')
-  error('GNU grep not found')
-endif
   endif
 elif host_machine.system() == 'darwin'
   grep_prog = find_program('ggrep')

[PATCH resend] lib: Drop internal virXXXPtr typedefs

[Michal Privoznik]

Historically, we declared pointer type to our types:

  typedef struct _virXXX virXXX;
  typedef virXXX *virXXXPtr;

But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.

This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:

https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html

Signed-off-by: Michal Privoznik 
---

This is just a resend of an updated patch I've sent earlier:

https://listman.redhat.com/archives/libvir-list/2021-March/msg00543.html

For full patch refer to my gitlab branch:

https://gitlab.com/MichalPrivoznik/libvirt/-/commit/06e73c327ef4f85950e38726d9ea204f1ed19a0e


 docs/advanced-tests.rst   |2 +-
 docs/coding-style.rst |2 +-
 docs/internals/command.html.in|   12 +-
 docs/internals/rpc.html.in|   32 +-
 scripts/esx_vi_generator.py   |6 +-
 scripts/hyperv_wmi_generator.py   |6 +-
 src/access/viraccessdriver.h  |   52 +-
 src/access/viraccessdrivernop.c   |   46 +-
 src/access/viraccessdriverpolkit.c|   52 +-
 src/access/viraccessdriverstack.c |   82 +-
 src/access/viraccessdriverstack.h |4 +-
 src/access/viraccessmanager.c |   78 +-
 src/access/viraccessmanager.h |   57 +-
 src/admin/admin_remote.c  |   56 +-
 src/admin/admin_server.c  |   34 +-
 src/admin/admin_server.h  |   26 +-
 src/admin/admin_server_dispatch.c |  129 +-
 src/admin/admin_server_dispatch.h |8 +-
 src/admin/libvirt-admin.c |8 +-
 src/bhyve/bhyve_capabilities.c|   26 +-
 src/bhyve/bhyve_capabilities.h|8 +-
 src/bhyve/bhyve_command.c |  124 +-
 src/bhyve/bhyve_command.h |   14 +-
 src/bhyve/bhyve_conf.c|   18 +-
 src/bhyve/bhyve_conf.h|9 +-
 src/bhyve/bhyve_device.c  |   30 +-
 src/bhyve/bhyve_device.h  |6 +-
 src/bhyve/bhyve_domain.c  |   36 +-
 src/bhyve/bhyve_domain.h  |7 +-
 src/bhyve/bhyve_driver.c  |  162 +-
 src/bhyve/bhyve_driver.h  |6 +-
 src/bhyve/bhyve_monitor.c |   34 +-
 src/bhyve/bhyve_monitor.h |7 +-
 src/bhyve/bhyve_parse_command.c   |   40 +-
 src/bhyve/bhyve_parse_command.h   |4 +-
 src/bhyve/bhyve_process.c |   52 +-
 src/bhyve/bhyve_process.h |   16 +-
 src/bhyve/bhyve_utils.h   |   25 +-
 src/conf/backup_conf.c|   54 +-
 src/conf/backup_conf.h|   24 +-
 src/conf/capabilities.c   |  240 +-
 src/conf/capabilities.h   |  116 +-
 src/conf/checkpoint_conf.c|   80 +-
 src/conf/checkpoint_conf.h|   23 +-
 src/conf/cpu_conf.c   |   76 +-
 src/conf/cpu_conf.h   |   69 +-
 src/conf/device_conf.c|   36 +-
 src/conf/device_conf.h|   42 +-
 src/conf/domain_addr.c|  316 +--
 src/conf/domain_addr.h|  128 +-
 src/conf/domain_audit.c   |   96 +-
 src/conf/domain_audit.h   |   76 +-
 src/conf/domain_capabilities.c|   70 +-
 src/conf/domain_capabilities.h|   39 +-
 src/conf/domain_conf.c| 2517 -
 src/conf/domain_conf.h|  967 ---
 src/conf/domain_event.c   |  552 ++--
 src/conf/domain_event.h   |  178 +-
 src/conf/domain_nwfilter.c|   18 +-
 src/conf/domain_nwfilter.h|6 +-
 src/conf/domain_validate.c|   64 +-
 src/conf/domain_validate.h|   10 +-
 src/conf/interface_conf.c |   76 +-
 src/conf/interface_conf.h |   18 +-
 src/conf/moment_conf.c|8 +-
 src/conf/moment_conf.h|8 +-
 src/conf/netdev_bandwidth_conf.c  |   12 +-
 src/conf/netdev_bandwidth_conf.h  |6 +-
 src/conf/netdev_vlan_conf.c   |4 +-
 src/conf/netdev_vlan_conf.h   |4 +-

Re: [libvirt PATCH 1/4] meson: Print custom message when GNU grep is not installed

[Erik Skultety]

On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
> Currently, if GNU grep is not installed on a FreeBSD system the
> configuration step will fail with
> 
>   Program grep found: YES (/usr/bin/grep)
>   Program /usr/local/bin/grep found: NO
> 
>   ERROR: Program '/usr/local/bin/grep' not found
> 
> which is confusing and not very useful; after this change, the
> message will be
> 
>   Program grep found: YES (/usr/bin/grep)
>   Program /usr/local/bin/grep found: NO
> 
>   ERROR: Problem encountered: GNU grep not found
> 
> instead, which should do a better job helping the user figure
> out that they need to install GNU grep from ports to proceed.
> 
> Signed-off-by: Andrea Bolognani 
> ---
Reviewed-by: Erik Skultety 

Re: [PATCH 2/6] qemu: capabilities: Introduce QEMU_CAPS_COMPAT_DEPRECATED

[Martin Kletzander]

On Thu, Apr 01, 2021 at 10:07:15AM +0200, Peter Krempa wrote:

On Thu, Apr 01, 2021 at 00:05:55 +0200, Martin Kletzander wrote:

On Fri, Mar 19, 2021 at 07:33:50PM +0100, Peter Krempa wrote:
> The capability is asserted if qemu supports the -compat
> deprecated-input= and deprecated-output= settings to control what should
> happen if deprecated fields are used in QMP.
>
> This will be used for a developer/tester-oriented setting which will
> aid us in catching use of deprecated settings sooner.
>
> Signed-off-by: Peter Krempa 
> ---
> src/qemu/qemu_capabilities.c | 8 
> src/qemu/qemu_capabilities.h | 1 +
> tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index dc1b10cd66..beea57caf6 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -624,6 +624,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
>   "audiodev",
>   "blockdev-backup",
>   "object.qapified",
> +  "compat-deprecated",
> );
>
>
> @@ -5187,6 +5188,13 @@ virQEMUCapsInitProcessCapsInterlock(virQEMUCapsPtr 
qemuCaps)
>
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV))
> virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI);
> +
> +/* The -compat qemu command line argument is implemented using a newer
> + * method which doesn't show up in query-command-line-options. As we'll 
use
> + * it only for development and testing purposes we can base the 
capability
> + * on a not entirely related witness. */
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_QAPIFIED))
> +virQEMUCapsSet(qemuCaps, QEMU_CAPS_COMPAT_DEPRECATED);

Or it could be just enabled by default since it should be used by devs
and CI only.


The idea here is that the capability will be used to hide the option
from qemus which don't support it, so that you can e.g. downgrade your
qemu or switch from git and non-git versions wihout having to touch this
option.

qemu-5.2.0:

./qemu-system-x86_64 -compat
qemu-system-x86_64: -compat: invalid option



Oh, good point, I haven't thought of the option itself.


signature.asc
Description: PGP signature

Re: [PATCH 0/6] qemu: Allow control of deprecation behaviour

[Martin Kletzander]

On Thu, Apr 01, 2021 at 10:37:31AM +0200, Peter Krempa wrote:

On Thu, Apr 01, 2021 at 00:36:52 +0200, Martin Kletzander wrote:

On Fri, Mar 19, 2021 at 07:33:48PM +0100, Peter Krempa wrote:
> For debugging purposes it's very useful to disable all deprecated
> commands and fields in qemu. This series implements a qemu.conf knob and
> a qemu namespace element to control this.
>
> The implementation tries to be very conservative to allow downgrades of
> qemu and such without breaking the startup of the VM.
>
> The intention is that developers and CI deployments use the 'crash'
> option to catch any unexpected qemu disappearance.
>
> Note that this applies on top of my series for -object QAPIfication.
>
> Based on Markus' -compat series which was now merged to upstream qemu.
>
> Peter Krempa (6):
>  docs/drvqemu: Convert to RST

I wanted to review the output of this first patch side-by-side so that I
do not have to go through all the markup hell.  But when I tried looking
for the artefacts from any possible pipeline this could have run through
I was greeted with a "get off my lawn" message.  So if you are okay with
me just skimming through the first patch (after reviewing the rest of da
patch series), then with the mentioned fixes, this is for you (xkcd#276)


Oops!, despite criticising the same thing last time [1] I actually didn't
(forgot to?) push the branch to gitlab so that artifacts would be
present.

https://pipo.sk.gitlab.io/-/libvirt/-/jobs/1145757051/artifacts/website/drvqemu.html



=D looks good ;)



[1] https://listman.redhat.com/archives/libvir-list/2021-March/msg00602.html


signature.asc
Description: PGP signature

Re: [PATCH 0/6] qemu: Allow control of deprecation behaviour

[Peter Krempa]

On Thu, Apr 01, 2021 at 00:36:52 +0200, Martin Kletzander wrote:
> On Fri, Mar 19, 2021 at 07:33:48PM +0100, Peter Krempa wrote:
> > For debugging purposes it's very useful to disable all deprecated
> > commands and fields in qemu. This series implements a qemu.conf knob and
> > a qemu namespace element to control this.
> > 
> > The implementation tries to be very conservative to allow downgrades of
> > qemu and such without breaking the startup of the VM.
> > 
> > The intention is that developers and CI deployments use the 'crash'
> > option to catch any unexpected qemu disappearance.
> > 
> > Note that this applies on top of my series for -object QAPIfication.
> > 
> > Based on Markus' -compat series which was now merged to upstream qemu.
> > 
> > Peter Krempa (6):
> >  docs/drvqemu: Convert to RST
> 
> I wanted to review the output of this first patch side-by-side so that I
> do not have to go through all the markup hell.  But when I tried looking
> for the artefacts from any possible pipeline this could have run through
> I was greeted with a "get off my lawn" message.  So if you are okay with
> me just skimming through the first patch (after reviewing the rest of da
> patch series), then with the mentioned fixes, this is for you (xkcd#276)

Oops!, despite criticising the same thing last time [1] I actually didn't
(forgot to?) push the branch to gitlab so that artifacts would be
present.

https://pipo.sk.gitlab.io/-/libvirt/-/jobs/1145757051/artifacts/website/drvqemu.html


[1] https://listman.redhat.com/archives/libvir-list/2021-March/msg00602.html

Re: [libvirt PATCH 1/2] conf: add support for disk "rotation_rate" property

[Daniel P . Berrangé]

On Thu, Apr 01, 2021 at 10:18:11AM +0800, Han Han wrote:
> On Wed, Mar 31, 2021 at 5:51 PM Daniel P. Berrangé 
> wrote:
> 
> > This lets the app expose the virtual SCSI or IDE disks as solid state
> > devices by setting a rate of '1', or rotational media by setting a
> > rate between 1025 and 65534.
> >
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> >  docs/formatdomain.rst | 13 ++---
> >  docs/schemas/domaincommon.rng |  5 +
> >  src/conf/domain_conf.c| 11 +++
> >  src/conf/domain_conf.h|  1 +
> >  4 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 741130bf21..224f44a0a2 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -2372,7 +2372,7 @@ paravirtualized driver is specified via the ``disk``
> > element.
> > 
> >   
> > 
> > -   
> > +   
> > 
> >   
> >   
> > @@ -2385,7 +2385,7 @@ paravirtualized driver is specified via the ``disk``
> > element.
> >  > mode='client'/>
> >   
> > 
> > -   
> > +   
> > 
> >   
> >   
> > @@ -2885,10 +2885,17 @@ paravirtualized driver is specified via the
> > ``disk`` element.
> > to "closed". NB, the value of ``tray`` could be updated while the
> > domain is
> > running. The optional attribute ``removable`` sets the removable flag
> > for USB
> > disks, and its value can be either "on" or "off", defaulting to "off".
> > +   The optional attribute ``rotation_rate`` sets the rotation rate of the
> > +   storage for disks on a SCSI, IDE, or SATA bus. Values in the range
> > 1025 to
> > +   65534 are used to indicate rotational media spee in revolutions per
> > minute.
> >
> I don't see any value range limitation in the libvirt code. Is it limited
> by qemu?

It isn't enforced by either, but these are the defined ranges in the
SCSI specs

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

Re: [PATCH 2/6] qemu: capabilities: Introduce QEMU_CAPS_COMPAT_DEPRECATED

[Peter Krempa]

On Thu, Apr 01, 2021 at 00:05:55 +0200, Martin Kletzander wrote:
> On Fri, Mar 19, 2021 at 07:33:50PM +0100, Peter Krempa wrote:
> > The capability is asserted if qemu supports the -compat
> > deprecated-input= and deprecated-output= settings to control what should
> > happen if deprecated fields are used in QMP.
> > 
> > This will be used for a developer/tester-oriented setting which will
> > aid us in catching use of deprecated settings sooner.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> > src/qemu/qemu_capabilities.c | 8 
> > src/qemu/qemu_capabilities.h | 1 +
> > tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
> > 3 files changed, 10 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index dc1b10cd66..beea57caf6 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -624,6 +624,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> >   "audiodev",
> >   "blockdev-backup",
> >   "object.qapified",
> > +  "compat-deprecated",
> > );
> > 
> > 
> > @@ -5187,6 +5188,13 @@ virQEMUCapsInitProcessCapsInterlock(virQEMUCapsPtr 
> > qemuCaps)
> > 
> > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV))
> > virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI);
> > +
> > +/* The -compat qemu command line argument is implemented using a newer
> > + * method which doesn't show up in query-command-line-options. As 
> > we'll use
> > + * it only for development and testing purposes we can base the 
> > capability
> > + * on a not entirely related witness. */
> > +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_QAPIFIED))
> > +virQEMUCapsSet(qemuCaps, QEMU_CAPS_COMPAT_DEPRECATED);
> 
> Or it could be just enabled by default since it should be used by devs
> and CI only.

The idea here is that the capability will be used to hide the option
from qemus which don't support it, so that you can e.g. downgrade your
qemu or switch from git and non-git versions wihout having to touch this
option.

qemu-5.2.0:

./qemu-system-x86_64 -compat
qemu-system-x86_64: -compat: invalid option

Re: [libvirt PATCH 0/2] qemu: wire up support for rotation rate for disks

[Erik Skultety]

On Wed, Mar 31, 2021 at 10:50:24AM +0100, Daniel P. Berrangé wrote:
> By default QEMU doesn't report any rotation information to guests, so
> guests assume rotational media. This lets the user specify an explicit
> speed in RPM, or 1 for SSD. This may allow the user to achieve better
> performance for their virtual disks. Note, however, this doesn't mean
> that the guest should be given the same setting as the host storage.
> It is possible that better performance may be achieved with contrary
> settings from the host. Testing is required to determine this on a
> case by case basis.

Oh, cool that we're finally able to introspect the device capability :). Please
link the following BZ in your commits before pushing:
https://bugzilla.redhat.com/show_bug.cgi?id=1498955

Regards,
Erik