Re: [libvirt PATCH v2 0/7] ci: Drop Fedora 34, add Fedora 36
On Thu, Jun 09, 2022 at 07:51:45AM -0400, Andrea Bolognani wrote: > On Thu, Jun 09, 2022 at 08:11:17AM +0200, Erik Skultety wrote: > > On Tue, Jun 07, 2022 at 01:06:41PM +0200, Erik Skultety wrote: > > > On Tue, Jun 07, 2022 at 02:34:50AM -0700, Andrea Bolognani wrote: > > > > Can I push the remaining two patches now, or are the issues that made > > > > it not possible at the time still relevant? > > > > > > Yes, the problem still persists. > > > > FYI, once we merge [1] we can merge this one as well. > > [1] https://listman.redhat.com/archives/libvir-list/2022-June/232322.html > > Those are in now. Can you confirm I can push the remaining patches? Sure, to both: Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 0/7] ci: Drop Fedora 34, add Fedora 36
On Thu, Jun 09, 2022 at 08:11:17AM +0200, Erik Skultety wrote: > On Tue, Jun 07, 2022 at 01:06:41PM +0200, Erik Skultety wrote: > > On Tue, Jun 07, 2022 at 02:34:50AM -0700, Andrea Bolognani wrote: > > > Can I push the remaining two patches now, or are the issues that made > > > it not possible at the time still relevant? > > > > Yes, the problem still persists. > > FYI, once we merge [1] we can merge this one as well. > [1] https://listman.redhat.com/archives/libvir-list/2022-June/232322.html Those are in now. Can you confirm I can push the remaining patches? -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Thu, Jun 09, 2022 at 07:34:07AM -0400, Andrea Bolognani wrote:
> On Thu, Jun 09, 2022 at 12:46:38PM +0200, Erik Skultety wrote:
> > On Thu, Jun 09, 2022 at 11:07:57AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jun 09, 2022 at 06:01:34AM -0400, Andrea Bolognani wrote:
> > > > On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> > > > > FWIW we could alternatively update the submodules manually, but we'd
> > > > > have list
> > > > > them explicitly, IOW:
> > > > > $ git clone qemu ...
> > > > > $ cd qemu.git
> > > > > $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
> > > >
> > > > We could avoid hardcoding the names of the submodules by using
> > > > something along the lines of
> > > >
> > > > $ ./scripts/git-submodule.sh update $(git submodule | awk '{print
> > > > $2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
> > > >
> > > > A bit of a mouthful, but should be solid enough.
> > > >
> > > > > $ mkdir build && cd build
> > > > > $ ../configure ... --with-git-submodules=ignore
> > > >
> > > > Using
> > > >
> > > > --with-git-submodules=validate
> > > >
> > > > would work too, since we'd have updated the submodules beforehand.
> > >
> > > 'validate' will still cause QEMU to run git commands to check
> > > the submodule state, so I presume it'll still hit the problem
> > > of ownership.
> >
> > Yes, only the 'ignore' option value is viable here.
> >
> > > > I think I would prefer this approach to changing the git
> > > > configuration for the root user.
> > >
> > > I was going to say the opposite. Updating the root user git config
> > > is harmless since our integration suite is intended to always run
> > > inside a single use throwaway VM. IOW, we already assume the VM is
> > > compromised at the end of every test cycle.
> >
> > Yes, thanks for bringing it up, ^this is the main reason why I didn't
> > propose
> > the submodule approach in the first place.
>
> Alrighty,
>
> Reviewed-by: Andrea Bolognani
>
> to the original patch then :)
Pushed.
Thanks,
Erik
Re: [libvirt PATCH 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Thu, Jun 09, 2022 at 12:46:38PM +0200, Erik Skultety wrote:
> On Thu, Jun 09, 2022 at 11:07:57AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jun 09, 2022 at 06:01:34AM -0400, Andrea Bolognani wrote:
> > > On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> > > > FWIW we could alternatively update the submodules manually, but we'd
> > > > have list
> > > > them explicitly, IOW:
> > > > $ git clone qemu ...
> > > > $ cd qemu.git
> > > > $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
> > >
> > > We could avoid hardcoding the names of the submodules by using
> > > something along the lines of
> > >
> > > $ ./scripts/git-submodule.sh update $(git submodule | awk '{print
> > > $2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
> > >
> > > A bit of a mouthful, but should be solid enough.
> > >
> > > > $ mkdir build && cd build
> > > > $ ../configure ... --with-git-submodules=ignore
> > >
> > > Using
> > >
> > > --with-git-submodules=validate
> > >
> > > would work too, since we'd have updated the submodules beforehand.
> >
> > 'validate' will still cause QEMU to run git commands to check
> > the submodule state, so I presume it'll still hit the problem
> > of ownership.
>
> Yes, only the 'ignore' option value is viable here.
>
> > > I think I would prefer this approach to changing the git
> > > configuration for the root user.
> >
> > I was going to say the opposite. Updating the root user git config
> > is harmless since our integration suite is intended to always run
> > inside a single use throwaway VM. IOW, we already assume the VM is
> > compromised at the end of every test cycle.
>
> Yes, thanks for bringing it up, ^this is the main reason why I didn't propose
> the submodule approach in the first place.
Alrighty,
Reviewed-by: Andrea Bolognani
to the original patch then :)
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Thu, Jun 09, 2022 at 11:07:57AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 09, 2022 at 06:01:34AM -0400, Andrea Bolognani wrote:
> > On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> > > FWIW we could alternatively update the submodules manually, but we'd have
> > > list
> > > them explicitly, IOW:
> > > $ git clone qemu ...
> > > $ cd qemu.git
> > > $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
> >
> > We could avoid hardcoding the names of the submodules by using
> > something along the lines of
> >
> > $ ./scripts/git-submodule.sh update $(git submodule | awk '{print
> > $2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
> >
> > A bit of a mouthful, but should be solid enough.
> >
> > > $ mkdir build && cd build
> > > $ ../configure ... --with-git-submodules=ignore
> >
> > Using
> >
> > --with-git-submodules=validate
> >
> > would work too, since we'd have updated the submodules beforehand.
>
> 'validate' will still cause QEMU to run git commands to check
> the submodule state, so I presume it'll still hit the problem
> of ownership.
Yes, only the 'ignore' option value is viable here.
>
> > I think I would prefer this approach to changing the git
> > configuration for the root user.
>
> I was going to say the opposite. Updating the root user git config
> is harmless since our integration suite is intended to always run
> inside a single use throwaway VM. IOW, we already assume the VM is
> compromised at the end of every test cycle.
Yes, thanks for bringing it up, ^this is the main reason why I didn't propose
the submodule approach in the first place.
Erik
Re: [libvirt PATCH 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Thu, Jun 09, 2022 at 06:01:34AM -0400, Andrea Bolognani wrote:
> On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> > FWIW we could alternatively update the submodules manually, but we'd have
> > list
> > them explicitly, IOW:
> > $ git clone qemu ...
> > $ cd qemu.git
> > $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
>
> We could avoid hardcoding the names of the submodules by using
> something along the lines of
>
> $ ./scripts/git-submodule.sh update $(git submodule | awk '{print
> $2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
>
> A bit of a mouthful, but should be solid enough.
>
> > $ mkdir build && cd build
> > $ ../configure ... --with-git-submodules=ignore
>
> Using
>
> --with-git-submodules=validate
>
> would work too, since we'd have updated the submodules beforehand.
'validate' will still cause QEMU to run git commands to check
the submodule state, so I presume it'll still hit the problem
of ownership.
> I think I would prefer this approach to changing the git
> configuration for the root user.
I was going to say the opposite. Updating the root user git config
is harmless since our integration suite is intended to always run
inside a single use throwaway VM. IOW, we already assume the VM is
compromised at the end of every test cycle.
With 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 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> FWIW we could alternatively update the submodules manually, but we'd have list
> them explicitly, IOW:
> $ git clone qemu ...
> $ cd qemu.git
> $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
We could avoid hardcoding the names of the submodules by using
something along the lines of
$ ./scripts/git-submodule.sh update $(git submodule | awk '{print
$2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
A bit of a mouthful, but should be solid enough.
> $ mkdir build && cd build
> $ ../configure ... --with-git-submodules=ignore
Using
--with-git-submodules=validate
would work too, since we'd have updated the submodules beforehand.
I think I would prefer this approach to changing the git
configuration for the root user.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 1/2] ci: integration: SELinux relabel the QEMU we installed from git
On Wed, Jun 08, 2022 at 07:59:36AM +0200, Erik Skultety wrote: > Signed-off-by: Erik Skultety > --- > ci/integration-template.yml | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v3 12/15] virsh: Wire up new virDomainSetIOThreadParams parameters
On Wed, Jun 08, 2022 at 15:43:06 +0200, Michal Privoznik wrote: > Since virsh implements a wrapper over virDomainSetIOThreadParams() > (command iothreadset) let's wire up new typed parameters there too. > > Signed-off-by: Michal Privoznik > --- > docs/manpages/virsh.rst | 13 - > tools/virsh-domain.c| 24 +++- > 2 files changed, 35 insertions(+), 2 deletions(-) Reviewed-by: Peter Krempa
Re: [PATCH v3 11/15] qemu: Wire up new virDomainSetIOThreadParams parameters
On Wed, Jun 08, 2022 at 15:43:05 +0200, Michal Privoznik wrote: > Introduced in previous commit, QEMU driver needs to be taught how > to set VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and > VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX parameters on given IOThread. > Fortunately, this is fairly trivial to do and since these two > parameters are exposed in domain XML too the update of inactive > XML can be wired up too. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_driver.c | 140 +-- > src/qemu/qemu_monitor.h | 4 + > src/qemu/qemu_monitor_json.c | 2 + > 3 files changed, 141 insertions(+), 5 deletions(-) Reviewed-by: Peter Krempa
Re: [PATCH v3 06/15] conf: Introduce thread_pool_min and thread_pool_max attributes to IOThread
On Wed, Jun 08, 2022 at 15:43:00 +0200, Michal Privoznik wrote: > At least in case of QEMU an IOThread is actually a pool of > threads (see iothread_set_aio_context_params() in QEMU's code > base). As such, it can have minimal and maximal number of worker > threads. Allow setting them in domain XML. > > Signed-off-by: Michal Privoznik > --- > docs/formatdomain.rst | 6 +- > src/conf/domain_conf.c| 34 ++- > src/conf/domain_conf.h| 3 + > src/conf/domain_validate.c| 28 + > src/conf/schemas/domaincommon.rng | 10 +++ > .../iothreads-ids-pool-sizes.xml | 61 +++ > ...iothreads-ids-pool-sizes.x86_64-latest.xml | 1 + > tests/qemuxml2xmltest.c | 1 + > 8 files changed, 140 insertions(+), 4 deletions(-) > create mode 100644 tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml > create mode 12 > tests/qemuxml2xmloutdata/iothreads-ids-pool-sizes.x86_64-latest.xml Reviewed-by: Peter Krempa
