On Mon, Feb 06, 2023 at 01:26:09PM +0600, Oleg Vasilev wrote:
>
>
> On 03.02.2023 19:45, Martin Kletzander wrote:
> > On Mon, Jan 30, 2023 at 09:00:01PM +0600, Oleg Vasilev wrote:
> > > Before, logs from deleted machines have been piling up, since there were
> > > no garbage collection
When handling virConnectOpen(), we parse given URI, specifically
all those parameters we know, like ?mode, ?socket, ?name, etc.
ignoring those we don't recognize yet. Then, we reconstruct the
URI back, but ignoring all parameters we've parsed. In other
words:
qemu:///system?mode=legacy=bar
Our own coding style suggest not inventing new names for labels
and stick with 'cleanup' (when the path is used in both,
successful and unsuccessful returns), or 'error' (when the code
below the label is used only upon error). Well, 'failed' label
falls into the latter category. Rename it then.
The aim of this helper is to manipulate the .ignore value for
given list of parameters. For instance:
virURIParamsSetIgnore(uri, false, {"mode", "socket", NULL});
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 1 +
src/util/viruri.c| 18 ++
Almost in all places where an URI is parsed we look for
additional argument(s). The remote driver's parsing uses two
macros EXTRACT_URI_ARG_STR() and EXTRACT_URI_ARG_BOOL() for that
purpose. Expose these so that other places can be rewritten using
those macros.
Signed-off-by: Michal Privoznik
Similarly to the previous commit, let's accept "socket" parameter
in the connection URI. This change will allow us to use
virt-ssh-helper instead of 'nc' in all cases (done in one of
future commits).
Please note, when the parameter is used it effectively disables
automatic daemon spawning and an
Even though we have split daemons for some time now, they are not
the default by far. We've made the change ~1.5 year ago (in
v7.5.0-rc1~35).
Therefore, we have some users that use 'mode=legacy' parameter in
their connection URI. But this parameter is not propagated to
virt-ssh-helper (and
The first couple of patches are cleanups, mostly. The last 5 patches are
the important ones. Now, the fix I went with in the 10/10 is to format
URI anew, just for the virt-ssh-helper's sake. I did not want to touch
@name as it's passed to sub-daemon's .open() method. If desired, I can
change the
Our URI handling code (doRemoteOpen() specifically), uses case
insensitive parsing of query part of URI. For instance:
qemu:///system?socket=/some/path
qemu:///system?SoCkEt=/some/path
are the same URI. Even though the latter is probably not used
anywhere, let's switch to STRCASEEQ() instead
The virURIFormat() function either returns a string, or aborts
(on OOM). There's no way this function can return NULL (as of
v7.2.0-rc1~277). Therefore, it doesn't make sense to check its
retval against NULL.
Signed-off-by: Michal Privoznik
---
src/admin/libvirt-admin.c | 6 +-
Now that we have VIR_EXTRACT_URI_ARG_* macros, we can use them in
the rest of the places where an URI is parsed. This also unifies
behavior wrt to query arguments handling. For instance, our
virAdmConnectOpen() accepts "?socket=..." but not "?SOCKET="
whereas plain virConnectOpen() does accept
When split daemons were introduced, we also made connection URI
accept new parameter: mode={auto,legacy,direct} so that a client
can force connecting to either old, monolithic daemon, or to
split daemon (see v5.7.0-rc1~257 for more info).
Now, the change was done to the remote driver, but not to
On 2/2/23 17:10, Peter Krempa wrote:
> Some more patches from old branches that I didn't get around finishing
> before.
>
> Peter Krempa (5):
> virbitmap: Allow NULL bitmap in functions returning index of a
> set/clear bit
> qemuMonitorJSONQueryStats: Simplify logic to construct
On Mon, Feb 06, 2023 at 03:32:20PM +0100, Peter Krempa wrote:
On Mon, Feb 06, 2023 at 15:28:29 +0100, Martin Kletzander wrote:
This reverts commit f2d379e7cb802f922409c35e4831ee52a2162486. On top of that it
also removes the `/tags` file because we don't even have the `make tags` target
any
On Mon, Feb 06, 2023 at 15:38:11 +0100, Martin Kletzander wrote:
> On Mon, Feb 06, 2023 at 03:32:20PM +0100, Peter Krempa wrote:
> > On Mon, Feb 06, 2023 at 15:28:29 +0100, Martin Kletzander wrote:
> > > This reverts commit f2d379e7cb802f922409c35e4831ee52a2162486. On top of
> > > that it
> > >
On Mon, Feb 06, 2023 at 03:40:44PM +0100, Peter Krempa wrote:
On Mon, Feb 06, 2023 at 15:38:11 +0100, Martin Kletzander wrote:
On Mon, Feb 06, 2023 at 03:32:20PM +0100, Peter Krempa wrote:
> On Mon, Feb 06, 2023 at 15:28:29 +0100, Martin Kletzander wrote:
> > This reverts commit
On Mon, Feb 06, 2023 at 10:16:50 +0100, Michal Privoznik wrote:
> The virURIFormat() function either returns a string, or aborts
> (on OOM). There's no way this function can return NULL (as of
> v7.2.0-rc1~277). Therefore, it doesn't make sense to check its
> retval against NULL.
>
>
On Mon, Feb 06, 2023 at 10:16:51 +0100, Michal Privoznik wrote:
> Our own coding style suggest not inventing new names for labels
> and stick with 'cleanup' (when the path is used in both,
> successful and unsuccessful returns), or 'error' (when the code
> below the label is used only upon error).
Signed-off-by: Erik Skultety
---
.gitlab-ci.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e20d0b9be8..921b04cd7b 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -30,8 +30,8 @@ include:
- meson dist -C build --no-tests
Even though 'setup' is assumed when no other command is given, we're
being explicit in our GitLab recipes, so do the same for the local
build.sh script too.
Signed-off-by: Erik Skultety
---
ci/build.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ci/build.sh b/ci/build.sh
build.sh is not the place where this should be mentioned as the
official entrypoint for this script locally is ci/helper which can
download the right image from our upstream CI registry. Since the idea
is to ultimately drop the usage of a Makefile for the local executions,
this patch doesn't
There's no harm in always building in system mode, i.e. setting the
right paths.
Signed-off-by: Erik Skultety
---
ci/build.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ci/build.sh b/ci/build.sh
index c7cba6ffa8..f6db4d2a7f 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@
Although it is currently consistent with the other variables we define
when running ci in a local container environment, it isn't consistent
with the variable naming we use in GitLab recipes. Since the idea is
to unite the two, we're likely going to drop a few other variables from
the local env
This change however involves adding a couple of new environment
variables as well as tuning the helper script to support local
container executions properly.
The overall motivation here is to move all script logic from
.gitlab-ci.yml to the build.sh script so that the steps are consistent
and
Future patches will add more functions corresponding to the behaviour
we define in individual GitLab jobs in .gitlab-ci.yml. This is just
a preliminary patch.
Signed-off-by: Erik Skultety
---
ci/build.sh | 23 +++
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git
This is a follow up to:
https://listman.redhat.com/archives/libvir-list/2023-January/237201.html
The effort here is to unify the way builds/tests are executed in GitLab CI vs
local container executions and make another step forward in terms of
reproducibility of (specifically) GitLab
This variable is specific to local container execution and is a result
of the hierarchy we chose for local container executions:
$ ls
build libvirt # build is the script, libvirt is a repo clone
The variable would never be populated in GitLab environment, so we set
the default to $PWD to
Instead of open-coding the script steps like we've done until now, use
a shell script.
Signed-off-by: Erik Skultety
---
.gitlab-ci.yml | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index
This would otherwise collide with local executions where we
1) don't collect artifacts
2) are not limited by GitLab's environment and hence moving build
artifacts to unusual places would only cause confusion when doing
local build inspection
Signed-off-by: Erik Skultety
---
.gitlab-ci.yml
This is the default setting in GitLab container environments and it
makes sense to stop executing the script with the first error
encountered.
Signed-off-by: Erik Skultety
---
ci/build.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/ci/build.sh b/ci/build.sh
index 4d7ef810f2..b6596300be
It is quite confusing seeing these two in a call like this one:
$ meson build $MESON_OPTS $MESON_ARGS
One has to ask 'how are they different' and 'shouldn't these be
merged'. In fact, these variables hold very different things and we
should make it more obvious. The problem is that renaming
This is continuation of what commit b56e2be68e3 started. If we stick to
only calling meson commands directly, we can achieve much better
consistency in passing arguments to meson especially if we unify the
recipes run in gitlab CI and what we can currently run locally in
containers using
Now that we have the GitLab job helper functions in place, we can wire
them up to the CLI interface.
Signed-off-by: Erik Skultety
---
ci/Makefile | 5 ++---
ci/build.sh | 39 +--
2 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/ci/Makefile
Introduce more helper functions corresponding to the jobs we currently
run for container workloads in GitLab. Some of the variables used in
the functions have to provide default values identical to the options
we pass to the jobs in GitLab to match the same behaviour if not
overriden by the user
You can specify multiple targets at once for the 'compile' command.
Signed-off-by: Erik Skultety
---
.gitlab-ci.yml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 699be460ca..e20d0b9be8 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
Unless we run it as 'sh ci/build.sh' in .gitlab-ci.yml recipes and
instead opt into doing 'chmod +x && ci/build.sh' it will cause meson
dist build to fail with a fatal error about having uncommitted changes
in the repo. Therefore, let's just make the script executable, it's the
most
Meson dist build is unhappy with the git clone we mount into local
container environments and forces updating git's index.
Since this is only relevant to the dist build, only update the index
then.
Signed-off-by: Erik Skultety
---
Honestly I have no good explanation why dist kept complaining
ci/build.sh is already exporting all of those and there's no need for
these variables to necessarily be defined and exported by GitLab
explicitly, this can be transparent to the job definition plus it keeps
everything important in a single place.
Signed-off-by: Erik Skultety
---
.gitlab-ci.yml
Originally coming from the list in our gitlab-ci.yml. The corresponding
gitlab-ci.yml change will come in a future patch.
Signed-off-by: Erik Skultety
---
ci/build.sh | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/ci/build.sh b/ci/build.sh
index
On a Friday in 2023, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (3):
qemuProcessStop: Fix detection of outgoing migration for external
devices
qemuExtTPMStop: Restore TPM state label more often
qemuProcessLaunch: Tighten rules for external devices wrt incoming
This reverts commit f2d379e7cb802f922409c35e4831ee52a2162486. On top of that it
also removes the `/tags` file because we don't even have the `make tags` target
any more. Any tool-related ignores should go to user's global ignore file or
the user's local exclude file which is per-project. See
On Mon, Feb 06, 2023 at 04:08:26PM +0100, Michal Privoznik wrote:
While the first one qualifies to be pushed as trivial, the second less
so. I'll wait a while and if there's no reply I'll just push these, as
the build is currently broken.
Michal Prívozník (2):
virhostdevtest: Initialize
On a Monday in 2023, Michal Privoznik wrote:
With recent work on storing original PCI stats in
_virDomainHostdevSubsysPCI struct, the virhostdevtest can across
can across? :)
a latent bug we had. Only some parts of the
virDomainHostdevSubsys structure are initialized. Incidentally,
On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote:
> This is a follow up to:
> https://listman.redhat.com/archives/libvir-list/2023-January/237201.html
>
> The effort here is to unify the way builds/tests are executed in GitLab CI vs
> local container executions and make another step
On Fri, Jan 27, 2023 at 10:26:48AM +0100, Erik Skultety wrote:
> Using shell scripts rather than inlining shell commands to YAML feels more
> natural, more readable, and will keep all different variations of execution
> consistent. Essentially the only disadvantage is that we won't see each
>
On Mon, Feb 06, 2023 at 01:26:09PM +0600, Oleg Vasilev wrote:
On 03.02.2023 19:45, Martin Kletzander wrote:
On Mon, Jan 30, 2023 at 09:00:01PM +0600, Oleg Vasilev wrote:
Before, logs from deleted machines have been piling up, since there were
no garbage collection mechanism. Now, virtlogd
On Mon, Feb 06, 2023 at 10:16:49 +0100, Michal Privoznik wrote:
> Our URI handling code (doRemoteOpen() specifically), uses case
> insensitive parsing of query part of URI. For instance:
>
> qemu:///system?socket=/some/path
> qemu:///system?SoCkEt=/some/path
>
> are the same URI. Even though
While the first one qualifies to be pushed as trivial, the second less
so. I'll wait a while and if there's no reply I'll just push these, as
the build is currently broken.
Michal Prívozník (2):
virhostdevtest: Initialize hostdev @subsys
virhostdevtest: Decrease possibility of uninitialized
With recent work on storing original PCI stats in
_virDomainHostdevSubsysPCI struct, the virhostdevtest can across
a latent bug we had. Only some parts of the
virDomainHostdevSubsys structure are initialized. Incidentally,
subsys->u.pci.origstates is not one of them. This lead to
unexpected
With the current way the myInit() is written, it's fairly easy to
miss initialization of @subsys variable as the variable is
allocated firstly on the stack and then it's assigned to
hostdev[i] which was allocated using g_new0() (this it is
containing nothing but all zeroes).
Make the subsys point
On Fri, Feb 03, 2023 at 06:13:23PM -0500, Laine Stump wrote:
On 2/3/23 2:49 AM, Erik Skultety wrote:
On Thu, Feb 02, 2023 at 02:02:13PM -0500, Laine Stump wrote:
On 2/2/23 10:37 AM, Martin Kletzander wrote:
Commit f7114e61dbc2 cleaned up way too much and now that I have cscope
working again I
On Mon, Feb 06, 2023 at 03:45:12PM +, Daniel P. Berrangé wrote:
> On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote:
> > This is a follow up to:
> > https://listman.redhat.com/archives/libvir-list/2023-January/237201.html
> >
> > The effort here is to unify the way builds/tests
On Mon, Feb 06, 2023 at 15:28:29 +0100, Martin Kletzander wrote:
> This reverts commit f2d379e7cb802f922409c35e4831ee52a2162486. On top of that
> it
> also removes the `/tags` file because we don't even have the `make tags`
> target
> any more. Any tool-related ignores should go to user's
Similar to other error paths in qemuDomainUnshareNamespace(), jump to
the cleanup label on umount error instead of directly returning -1.
Signed-off-by: Jim Fehlig
---
I noticed this while looking at a bug report containing the error. ATM I'm not
sure why the umount failed, but have asked for
On 2/6/23 16:02, Peter Krempa wrote:
> On Mon, Feb 06, 2023 at 10:16:49 +0100, Michal Privoznik wrote:
>> Our URI handling code (doRemoteOpen() specifically), uses case
>> insensitive parsing of query part of URI. For instance:
>>
>> qemu:///system?socket=/some/path
>>
55 matches
Mail list logo
