[libvirt] [jenkins-ci PATCH 1/6] mappings: add libgcrypt native & mingw packages

2019-05-03 Thread Daniel P . Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 guests/vars/mappings.yml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 98c0cc9..7cde719 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -319,6 +319,11 @@ mappings:
 rpm: dbus-devel
 cross-policy-deb: foreign
 
+  libgcrypt:
+rpm: libgcrypt-devel
+deb: libgcrypt20-dev
+cross-policy-deb: foreign
+
   libgovirt:
 rpm: libgovirt-devel
 Debian: libgovirt-dev
@@ -481,6 +486,9 @@ mappings:
   mingw32-libarchive:
 FedoraRawhide: mingw32-libarchive
 
+  mingw32-libgcrypt:
+FedoraRawhide: mingw32-libgcrypt
+
   mingw32-libgovirt:
 FedoraRawhide: mingw32-libgovirt
 
@@ -559,6 +567,9 @@ mappings:
   mingw64-libarchive:
 FedoraRawhide: mingw64-libarchive
 
+  mingw64-libgcrypt:
+FedoraRawhide: mingw64-libgcrypt
+
   mingw64-libgovirt:
 FedoraRawhide: mingw64-libgovirt
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 4/6] projects: make virt-viewer depend on gtk-vnc jobs

2019-05-03 Thread Daniel P . Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 guests/playbooks/build/jobs/defaults.yml | 1 +
 guests/vars/projects/virt-viewer+mingw32.yml | 1 -
 guests/vars/projects/virt-viewer+mingw64.yml | 1 -
 guests/vars/projects/virt-viewer.yml | 1 -
 jenkins/jobs/defaults.yaml   | 1 +
 jenkins/projects/virt-viewer+mingw32.yaml| 4 +++-
 jenkins/projects/virt-viewer+mingw64.yaml| 4 +++-
 jenkins/projects/virt-viewer.yaml| 4 +++-
 8 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/guests/playbooks/build/jobs/defaults.yml 
b/guests/playbooks/build/jobs/defaults.yml
index 8a9eebc..f43ebd0 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -26,6 +26,7 @@ strip_buildrequires: |
   sed -i -e 's/BuildRequires: *libvirt.*//' *.spec*
   sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec*
   sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec*
+  sed -i -e 's/BuildRequires: *pkgconfig(gtk-vnc-.*).*//' *.spec*
   sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec*
 mingw32_local_env: |
   export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
diff --git a/guests/vars/projects/virt-viewer+mingw32.yml 
b/guests/vars/projects/virt-viewer+mingw32.yml
index 2b914c3..608d722 100644
--- a/guests/vars/projects/virt-viewer+mingw32.yml
+++ b/guests/vars/projects/virt-viewer+mingw32.yml
@@ -6,7 +6,6 @@ packages:
   - mingw32-gstreamer1-plugins-bad-free
   - mingw32-gstreamer1-plugins-good
   - mingw32-gtk3
-  - mingw32-gtk-vnc2
   - mingw32-libgovirt
   - mingw32-libusbx
   - mingw32-rest
diff --git a/guests/vars/projects/virt-viewer+mingw64.yml 
b/guests/vars/projects/virt-viewer+mingw64.yml
index 6b42a7f..3aa5893 100644
--- a/guests/vars/projects/virt-viewer+mingw64.yml
+++ b/guests/vars/projects/virt-viewer+mingw64.yml
@@ -6,7 +6,6 @@ packages:
   - mingw64-gstreamer1-plugins-bad-free
   - mingw64-gstreamer1-plugins-good
   - mingw64-gtk3
-  - mingw64-gtk-vnc2
   - mingw64-libgovirt
   - mingw64-libusbx
   - mingw64-rest
diff --git a/guests/vars/projects/virt-viewer.yml 
b/guests/vars/projects/virt-viewer.yml
index cd32176..6ce638c 100644
--- a/guests/vars/projects/virt-viewer.yml
+++ b/guests/vars/projects/virt-viewer.yml
@@ -1,7 +1,6 @@
 ---
 packages:
   - glib2
-  - gtk-vnc2
   - gtk3
   - intltool
   - libgovirt
diff --git a/jenkins/jobs/defaults.yaml b/jenkins/jobs/defaults.yaml
index 7fb6d68..f3ba850 100644
--- a/jenkins/jobs/defaults.yaml
+++ b/jenkins/jobs/defaults.yaml
@@ -26,6 +26,7 @@
   sed -i -e 's/BuildRequires: *libvirt.*//' *.spec*
   sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec*
   sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec*
+  sed -i -e 's/BuildRequires: *pkgconfig(gtk-vnc-.*).*//' *.spec*
   sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec*
 mingw32_local_env: |
   export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
diff --git a/jenkins/projects/virt-viewer+mingw32.yaml 
b/jenkins/projects/virt-viewer+mingw32.yaml
index c9c74ea..7d4738a 100644
--- a/jenkins/projects/virt-viewer+mingw32.yaml
+++ b/jenkins/projects/virt-viewer+mingw32.yaml
@@ -7,6 +7,8 @@
 git_url: '{git_urls[virt-viewer][default]}'
 jobs:
   - autotools-build-job:
-  parent_jobs: 'libvirt-glib+mingw32-build'
+  parent_jobs:
+- 'libvirt-glib+mingw32-build'
+- 'gtk-vnc+mingw32-build'
   local_env: '{mingw32_local_env}'
   autogen_args: '{mingw32_autogen_args}'
diff --git a/jenkins/projects/virt-viewer+mingw64.yaml 
b/jenkins/projects/virt-viewer+mingw64.yaml
index c3b570f..dfbd70d 100644
--- a/jenkins/projects/virt-viewer+mingw64.yaml
+++ b/jenkins/projects/virt-viewer+mingw64.yaml
@@ -7,6 +7,8 @@
 git_url: '{git_urls[virt-viewer][default]}'
 jobs:
   - autotools-build-job:
-  parent_jobs: 'libvirt-glib+mingw64-build'
+  parent_jobs:
+- 'libvirt-glib+mingw64-build'
+- 'gtk-vnc+mingw64-build'
   local_env: '{mingw64_local_env}'
   autogen_args: '{mingw64_autogen_args}'
diff --git a/jenkins/projects/virt-viewer.yaml 
b/jenkins/projects/virt-viewer.yaml
index f06da31..87b5853 100644
--- a/jenkins/projects/virt-viewer.yaml
+++ b/jenkins/projects/virt-viewer.yaml
@@ -7,7 +7,9 @@
 git_url: '{git_urls[virt-viewer][default]}'
 jobs:
   - autotools-build-job:
-  parent_jobs: 'libvirt-glib-build'
+  parent_jobs:
+- 'libvirt-glib-build'
+- 'gtk-vnc-build'
   - autotools-syntax-check-job:
   parent_jobs: 'virt-viewer-build'
   - autotools-check-job:
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 5/6] mappings: remove gtk-vnc2 native and mingw packages

2019-05-03 Thread Daniel P . Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 guests/vars/mappings.yml | 12 
 1 file changed, 12 deletions(-)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 6347024..40eeb02 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -228,12 +228,6 @@ mappings:
   gtk-update-icon-cache:
 default: gtk-update-icon-cache
 
-  gtk-vnc2:
-deb: libgtk-vnc-2.0-dev
-pkg: gtk-vnc
-rpm: gtk-vnc2-devel
-cross-policy-deb: foreign
-
   hal:
 FreeBSD: hal
 
@@ -477,9 +471,6 @@ mappings:
   mingw32-gtk3:
 FedoraRawhide: mingw32-gtk3
 
-  mingw32-gtk-vnc2:
-FedoraRawhide: mingw32-gtk-vnc2
-
   mingw32-json-glib:
 FedoraRawhide: mingw32-json-glib
 
@@ -558,9 +549,6 @@ mappings:
   mingw64-gtk3:
 FedoraRawhide: mingw64-gtk3
 
-  mingw64-gtk-vnc2:
-FedoraRawhide: mingw64-gtk-vnc2
-
   mingw64-json-glib:
 FedoraRawhide: mingw64-json-glib
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 2/6] mappings: add pulseaudio libs for native packages

2019-05-03 Thread Daniel P . Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 guests/vars/mappings.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 7cde719..6347024 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -780,6 +780,10 @@ mappings:
 default: polkit
 deb: policykit-1
 
+  pulseaudio:
+rpm: pulseaudio-libs-devel
+deb: libpulse-dev
+
   python2:
 default: python
 Fedora: python2
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 6/6] jobs: allow test-suite.log to exist anywhere

2019-05-03 Thread Daniel P . Berrangé 
Don't assume that all projects have a dedicated tests/ subdirectory
for unit tests.

Signed-off-by: Daniel P. Berrangé 
---
 guests/playbooks/build/jobs/autotools-check-job.yml | 2 +-
 jenkins/jobs/autotools.yaml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/guests/playbooks/build/jobs/autotools-check-job.yml 
b/guests/playbooks/build/jobs/autotools-check-job.yml
index bcb6a85..a306902 100644
--- a/guests/playbooks/build/jobs/autotools-check-job.yml
+++ b/guests/playbooks/build/jobs/autotools-check-job.yml
@@ -9,7 +9,7 @@
 cd build
 if ! $MAKE check
 then
-cat tests/test-suite.log || true
+find -name test-suite.log -exec cat {} \;
 exit 1
 fi
   when:
diff --git a/jenkins/jobs/autotools.yaml b/jenkins/jobs/autotools.yaml
index f04ec17..72dd72e 100644
--- a/jenkins/jobs/autotools.yaml
+++ b/jenkins/jobs/autotools.yaml
@@ -126,7 +126,7 @@
   cd build
   if ! $MAKE check
   then
-  cat tests/test-suite.log || true
+  find -name test-suite.log -exec cat {} \;
   exit 1
   fi
 publishers:
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 3/6] projects: add gtk-vnc project

2019-05-03 Thread Daniel P . Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 guests/host_vars/libvirt-centos-7/main.yml|  1 +
 guests/host_vars/libvirt-debian-9/main.yml|  1 +
 guests/host_vars/libvirt-debian-sid/main.yml  |  1 +
 guests/host_vars/libvirt-fedora-29/main.yml   |  1 +
 guests/host_vars/libvirt-fedora-30/main.yml   |  1 +
 guests/host_vars/libvirt-fedora-rawhide/main.yml  |  3 +++
 guests/host_vars/libvirt-freebsd-11/main.yml  |  1 +
 guests/host_vars/libvirt-freebsd-12/main.yml  |  1 +
 guests/host_vars/libvirt-freebsd-current/main.yml |  1 +
 guests/host_vars/libvirt-ubuntu-18/main.yml   |  1 +
 guests/playbooks/build/jobs/defaults.yml  |  2 ++
 .../playbooks/build/projects/gtk-vnc+mingw32.yml  | 12 
 .../playbooks/build/projects/gtk-vnc+mingw64.yml  | 12 
 guests/playbooks/build/projects/gtk-vnc.yml   | 12 
 guests/vars/projects/gtk-vnc+mingw32.yml  |  6 ++
 guests/vars/projects/gtk-vnc+mingw64.yml  |  6 ++
 guests/vars/projects/gtk-vnc.yml  | 11 +++
 jenkins/jobs/defaults.yaml|  2 ++
 jenkins/projects/gtk-vnc+mingw32.yaml | 11 +++
 jenkins/projects/gtk-vnc+mingw64.yaml | 11 +++
 jenkins/projects/gtk-vnc.yaml | 15 +++
 21 files changed, 112 insertions(+)
 create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw32.yml
 create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw64.yml
 create mode 100644 guests/playbooks/build/projects/gtk-vnc.yml
 create mode 100644 guests/vars/projects/gtk-vnc+mingw32.yml
 create mode 100644 guests/vars/projects/gtk-vnc+mingw64.yml
 create mode 100644 guests/vars/projects/gtk-vnc.yml
 create mode 100644 jenkins/projects/gtk-vnc+mingw32.yaml
 create mode 100644 jenkins/projects/gtk-vnc+mingw64.yaml
 create mode 100644 jenkins/projects/gtk-vnc.yaml

diff --git a/guests/host_vars/libvirt-centos-7/main.yml 
b/guests/host_vars/libvirt-centos-7/main.yml
index fa4fc67..63e8501 100644
--- a/guests/host_vars/libvirt-centos-7/main.yml
+++ b/guests/host_vars/libvirt-centos-7/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-cim
diff --git a/guests/host_vars/libvirt-debian-9/main.yml 
b/guests/host_vars/libvirt-debian-9/main.yml
index ec7e6b4..b2b0b4d 100644
--- a/guests/host_vars/libvirt-debian-9/main.yml
+++ b/guests/host_vars/libvirt-debian-9/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-dbus
diff --git a/guests/host_vars/libvirt-debian-sid/main.yml 
b/guests/host_vars/libvirt-debian-sid/main.yml
index 1c7a29b..9c188cf 100644
--- a/guests/host_vars/libvirt-debian-sid/main.yml
+++ b/guests/host_vars/libvirt-debian-sid/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-dbus
diff --git a/guests/host_vars/libvirt-fedora-29/main.yml 
b/guests/host_vars/libvirt-fedora-29/main.yml
index bebf171..bdff061 100644
--- a/guests/host_vars/libvirt-fedora-29/main.yml
+++ b/guests/host_vars/libvirt-fedora-29/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-cim
diff --git a/guests/host_vars/libvirt-fedora-30/main.yml 
b/guests/host_vars/libvirt-fedora-30/main.yml
index 4ad27a6..bccc5fb 100644
--- a/guests/host_vars/libvirt-fedora-30/main.yml
+++ b/guests/host_vars/libvirt-fedora-30/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-cim
diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml 
b/guests/host_vars/libvirt-fedora-rawhide/main.yml
index ed0a3fa..9bd74e1 100644
--- a/guests/host_vars/libvirt-fedora-rawhide/main.yml
+++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml
@@ -1,5 +1,8 @@
 ---
 projects:
+  - gtk-vnc
+  - gtk-vnc+mingw32
+  - gtk-vnc+mingw64
   - libosinfo
   - libosinfo+mingw32
   - libosinfo+mingw64
diff --git a/guests/host_vars/libvirt-freebsd-11/main.yml 
b/guests/host_vars/libvirt-freebsd-11/main.yml
index ed805c9..0111b87 100644
--- a/guests/host_vars/libvirt-freebsd-11/main.yml
+++ b/guests/host_vars/libvirt-freebsd-11/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-dbus
diff --git a/guests/host_vars/libvirt-freebsd-12/main.yml 
b/guests/host_vars/libvirt-freebsd-12/main.yml
index 8bbe158..0c2e352 100644
--- a/guests/host_vars/libvirt-freebsd-12/main.yml
+++ b/guests/host_vars/libvirt-freebsd-12/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-dbus
diff --git a/guests/host_vars/libvirt-freebsd-current/main.yml 
b/guests/host_vars/libvirt-freebsd-current/main.yml
index 62498fd..a106ec0 100644
--- a/guests/host_vars/libvirt-freebsd-current/main.yml
+++ b/guests/host_vars/libvirt-freebsd-current/main.yml
@@ -1,5 +1,6 @@
 ---
 projects:
+  - gtk-vnc
   - libosinfo
   - libvirt
   - libvirt-dbus
diff --git a/guests/host_vars/libvirt

[libvirt] [jenkins-ci PATCH 0/6] Add support for gtk-vnc builds

2019-05-03 Thread Daniel P . Berrangé 
Support gtk-vnc and make virt-viewer depend on it

Daniel P. Berrangé (6):
  mappings: add libgcrypt native & mingw packages
  mappings: add pulseaudio libs for native packages
  projects: add gtk-vnc project
  projects: make virt-viewer depend on gtk-vnc jobs
  mappings: remove gtk-vnc2 native and mingw packages
  jobs: allow test-suite.log to exist anywhere

 guests/host_vars/libvirt-centos-7/main.yml|  1 +
 guests/host_vars/libvirt-debian-9/main.yml|  1 +
 guests/host_vars/libvirt-debian-sid/main.yml  |  1 +
 guests/host_vars/libvirt-fedora-29/main.yml   |  1 +
 guests/host_vars/libvirt-fedora-30/main.yml   |  1 +
 .../host_vars/libvirt-fedora-rawhide/main.yml |  3 +++
 guests/host_vars/libvirt-freebsd-11/main.yml  |  1 +
 guests/host_vars/libvirt-freebsd-12/main.yml  |  1 +
 .../libvirt-freebsd-current/main.yml  |  1 +
 guests/host_vars/libvirt-ubuntu-18/main.yml   |  1 +
 .../build/jobs/autotools-check-job.yml|  2 +-
 guests/playbooks/build/jobs/defaults.yml  |  3 +++
 .../build/projects/gtk-vnc+mingw32.yml| 12 +
 .../build/projects/gtk-vnc+mingw64.yml| 12 +
 guests/playbooks/build/projects/gtk-vnc.yml   | 12 +
 guests/vars/mappings.yml  | 27 ++-
 guests/vars/projects/gtk-vnc+mingw32.yml  |  6 +
 guests/vars/projects/gtk-vnc+mingw64.yml  |  6 +
 guests/vars/projects/gtk-vnc.yml  | 11 
 guests/vars/projects/virt-viewer+mingw32.yml  |  1 -
 guests/vars/projects/virt-viewer+mingw64.yml  |  1 -
 guests/vars/projects/virt-viewer.yml  |  1 -
 jenkins/jobs/autotools.yaml   |  2 +-
 jenkins/jobs/defaults.yaml|  3 +++
 jenkins/projects/gtk-vnc+mingw32.yaml | 11 
 jenkins/projects/gtk-vnc+mingw64.yaml | 11 
 jenkins/projects/gtk-vnc.yaml | 15 +++
 jenkins/projects/virt-viewer+mingw32.yaml |  4 ++-
 jenkins/projects/virt-viewer+mingw64.yaml |  4 ++-
 jenkins/projects/virt-viewer.yaml |  4 ++-
 30 files changed, 140 insertions(+), 20 deletions(-)
 create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw32.yml
 create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw64.yml
 create mode 100644 guests/playbooks/build/projects/gtk-vnc.yml
 create mode 100644 guests/vars/projects/gtk-vnc+mingw32.yml
 create mode 100644 guests/vars/projects/gtk-vnc+mingw64.yml
 create mode 100644 guests/vars/projects/gtk-vnc.yml
 create mode 100644 jenkins/projects/gtk-vnc+mingw32.yaml
 create mode 100644 jenkins/projects/gtk-vnc+mingw64.yaml
 create mode 100644 jenkins/projects/gtk-vnc.yaml

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 16:41 +0100, Daniel P. Berrangé wrote:
> On Fri, May 03, 2019 at 05:37:58PM +0200, Andrea Bolognani wrote:
> > > @@ -461,8 +462,12 @@ class Application:
> > >  "git_branch": git_branch,
> > >  })
> > >  
> > > +ap_path = distutils.spawn.find_executable("ansible-playbook")
> > 
> > Please call this 'ansible_playbook' instead of 'ap_path'...
> > 
> > > @@ -534,8 +539,12 @@ class Application:
> > >  # a kernel argument
> > >  extra_arg = "console=ttyS0 
> > > ks=file:/{}".format(install_config)
> > >  
> > > +vi_path = distutils.spawn.find_executable("virt-install")
> > 
> > ... and same here.
> 
> I'll call this one  "virt_install" rather than "ansible_playbook" ;-P

Since you insist, I'll begrudgingly accept this compromise :P

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH

2019-05-03 Thread Daniel P . Berrangé 
On Fri, May 03, 2019 at 05:37:58PM +0200, Andrea Bolognani wrote:
> On Fri, 2019-05-03 at 11:02 +0100, Daniel P. Berrangé wrote:
> > This improves error reporting:
> > 
> >   $ ./lcitool install libvirt-fedora-29
> >   ./lcitool: Failed to install 'libvirt-fedora-29': [Errno 2] No such file 
> > or directory
> > 
> > To
> > 
> >   $ ./lcitool install libvirt-fedora-29
> >   ./lcitool: Cannot find virt-install in $PATH
> 
> Much nicer indeed!
> 
> > @@ -461,8 +462,12 @@ class Application:
> >  "git_branch": git_branch,
> >  })
> >  
> > +ap_path = distutils.spawn.find_executable("ansible-playbook")
> 
> Please call this 'ansible_playbook' instead of 'ap_path'...
> 
> > @@ -534,8 +539,12 @@ class Application:
> >  # a kernel argument
> >  extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
> >  
> > +vi_path = distutils.spawn.find_executable("virt-install")
> 
> ... and same here.

I'll call this one  "virt_install" rather than "ansible_playbook" ;-P


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 11:02 +0100, Daniel P. Berrangé wrote:
> This improves error reporting:
> 
>   $ ./lcitool install libvirt-fedora-29
>   ./lcitool: Failed to install 'libvirt-fedora-29': [Errno 2] No such file or 
> directory
> 
> To
> 
>   $ ./lcitool install libvirt-fedora-29
>   ./lcitool: Cannot find virt-install in $PATH

Much nicer indeed!

> @@ -461,8 +462,12 @@ class Application:
>  "git_branch": git_branch,
>  })
>  
> +ap_path = distutils.spawn.find_executable("ansible-playbook")

Please call this 'ansible_playbook' instead of 'ap_path'...

> @@ -534,8 +539,12 @@ class Application:
>  # a kernel argument
>  extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
>  
> +vi_path = distutils.spawn.find_executable("virt-install")

... and same here.

Other than that, this works and AFAICT addresses all issues Pino
raised for v1, so

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH] lcitool: use yaml.safe_load instead of load

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 11:03 +0100, Daniel P. Berrangé wrote:
> The yaml.load() method is historically unsafe as it allowed for
> arbitrary code execution:
> 
> ./lcitool:323: YAMLLoadWarning: calling yaml.load() without
>  Loader=... is deprecated, as the default Loader is unsafe.
>  Please read https://msg.pyyaml.org/load for full details.
> 
> The PyYAML >= 5.1 is now safe by default, but has none the less
> deprecated the plain load() method to avoid risk for people
> running their app on older versions. For our needs safe_load()
> suffices and is compatible with RHEL-7
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  guests/lcitool | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH] lcitool: don't use SafeConfigParser under Python3

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 11:02 +0100, Daniel P. Berrangé wrote:
> Since Python 3.2 the SafeConfigParser class is renamed to
> just ConfigParser. The SafeConfigParser alias is targetted
> for deletion.
> 
>  ./lcitool:224: DeprecationWarning: The SafeConfigParser class
>   has been renamed to ConfigParser in Python 3.2. This alias
>   will be removed in future versions. Use ConfigParser directly
>   instead.
> 
> This switches to use ConfigParser all the time on Py3. Technically
> this is wrong on 3.0 and 3.1, but in reality no one will still be
> using those, only more modern 3.x versions, or the legacy 2.7.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  guests/lcitool | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] Avoid issues due to qemu dropping osxsave and ospke

2019-05-03 Thread Christian Ehrhardt 
On Thu, Apr 25, 2019 at 2:50 PM Christian Ehrhardt
 wrote:
>
> Hi,
> this series tries to address a drop of commandline options by qemu in regard 
> to
> osxsave [1] and ospke [2].
> This was already discussed in [3] late last year but got forgotten afterwards.
> The Ubuntu bug is at [4] and an older Fedora bug is at [5].
>
> TL;DR:
> - osxsave/ospke features were never really configurable
> - KVM never returned the bits on GET_SUPPORTED_CPUID
> - very rare to be seen in the wild
> - avoid issues with newer qemu and old/odd XMLs to be sure

Hi,
I know we are in a freeze, but we if one has time to review that would
be great to be ready when 5.4 opens.
But since 8 days passed I thought I could ping here to be seen (again).

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dockerfiles PATCH 2/3] Add Fedora 30

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 15:04 +0200, Pavel Hrdina wrote:
> On Fri, May 03, 2019 at 02:49:29PM +0200, Andrea Bolognani wrote:
> > +FROM fedora:30
> > +
> > +RUN yum update -y && \
> > +yum install -y audit-libs-devel \
> 
> We should stop using yum in Fedora and switch to dnf, yum is deprecated
> and there is a plan to remove it in Fedora 31.

Good point!

Do you feel like sending a patch? Alternatively, just remind me
next week :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dockerfiles PATCH 2/3] Add Fedora 30

2019-05-03 Thread Pavel Hrdina 
On Fri, May 03, 2019 at 02:49:29PM +0200, Andrea Bolognani wrote:
> The corresponding libvirt-jenkins-ci commit is b73a4dc8f6b9.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  buildenv-fedora-30.Dockerfile | 88 +++
>  1 file changed, 88 insertions(+)
>  create mode 100644 buildenv-fedora-30.Dockerfile
> 
> diff --git a/buildenv-fedora-30.Dockerfile b/buildenv-fedora-30.Dockerfile
> new file mode 100644
> index 000..d4d5da9
> --- /dev/null
> +++ b/buildenv-fedora-30.Dockerfile
> @@ -0,0 +1,88 @@
> +FROM fedora:30
> +
> +RUN yum update -y && \
> +yum install -y audit-libs-devel \

We should stop using yum in Fedora and switch to dnf, yum is deprecated
and there is a plan to remove it in Fedora 31.

Pavel

> +augeas \
> +autoconf \
> +automake \
> +avahi-devel \
> +bash \
> +bash-completion \
> +ca-certificates \
> +ccache \
> +chrony \
> +cppi \
> +cyrus-sasl-devel \
> +dbus-devel \
> +device-mapper-devel \
> +dnsmasq \
> +dwarves \
> +ebtables \
> +fuse-devel \
> +gcc \
> +gdb \
> +gettext \
> +gettext-devel \
> +git \
> +glibc-devel \
> +glusterfs-api-devel \
> +gnutls-devel \
> +iproute \
> +iproute-tc \
> +iscsi-initiator-utils \
> +kmod \
> +libacl-devel \
> +libattr-devel \
> +libblkid-devel \
> +libcap-ng-devel \
> +libcurl-devel \
> +libiscsi-devel \
> +libnl3-devel \
> +libpcap-devel \
> +libpciaccess-devel \
> +librbd-devel \
> +libselinux-devel \
> +libssh-devel \
> +libssh2-devel \
> +libtirpc-devel \
> +libtool \
> +libudev-devel \
> +libwsman-devel \
> +libxml2 \
> +libxml2-devel \
> +libxslt \
> +lsof \
> +lvm2 \
> +make \
> +ncurses-devel \
> +net-tools \
> +netcf-devel \
> +nfs-utils \
> +numactl-devel \
> +numad \
> +parted \
> +parted-devel \
> +patch \
> +perl \
> +pkgconfig \
> +polkit \
> +qemu-img \
> +radvd \
> +readline-devel \
> +rpcgen \
> +rpm-build \
> +sanlock-devel \
> +screen \
> +scrub \
> +sheepdog \
> +strace \
> +sudo \
> +systemtap-sdt-devel \
> +vim \
> +wireshark-devel \
> +xen-devel \
> +xfsprogs-devel \
> +yajl-devel \
> +zfs-fuse && \
> +yum autoremove -y && \
> +yum clean all -y
> -- 
> 2.20.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dockerfiles PATCH 3/3] Drop Fedora 28

2019-05-03 Thread Andrea Bolognani 
The corresponding libvirt-jenkins-ci commit is 3c305799f0f6.

Signed-off-by: Andrea Bolognani 
---
 buildenv-fedora-28.Dockerfile | 88 ---
 1 file changed, 88 deletions(-)
 delete mode 100644 buildenv-fedora-28.Dockerfile

diff --git a/buildenv-fedora-28.Dockerfile b/buildenv-fedora-28.Dockerfile
deleted file mode 100644
index 88fe63f..000
--- a/buildenv-fedora-28.Dockerfile
+++ /dev/null
@@ -1,88 +0,0 @@
-FROM fedora:28
-
-RUN yum update -y && \
-yum install -y audit-libs-devel \
-augeas \
-autoconf \
-automake \
-avahi-devel \
-bash \
-bash-completion \
-ca-certificates \
-ccache \
-chrony \
-cppi \
-cyrus-sasl-devel \
-dbus-devel \
-device-mapper-devel \
-dnsmasq \
-dwarves \
-ebtables \
-fuse-devel \
-gcc \
-gdb \
-gettext \
-gettext-devel \
-git \
-glibc-devel \
-glusterfs-api-devel \
-gnutls-devel \
-iproute \
-iproute-tc \
-iscsi-initiator-utils \
-kmod \
-libacl-devel \
-libattr-devel \
-libblkid-devel \
-libcap-ng-devel \
-libcurl-devel \
-libiscsi-devel \
-libnl3-devel \
-libpcap-devel \
-libpciaccess-devel \
-librbd-devel \
-libselinux-devel \
-libssh-devel \
-libssh2-devel \
-libtirpc-devel \
-libtool \
-libudev-devel \
-libwsman-devel \
-libxml2 \
-libxml2-devel \
-libxslt \
-lsof \
-lvm2 \
-make \
-ncurses-devel \
-net-tools \
-netcf-devel \
-nfs-utils \
-numactl-devel \
-numad \
-parted \
-parted-devel \
-patch \
-perl \
-pkgconfig \
-polkit \
-qemu-img \
-radvd \
-readline-devel \
-rpcgen \
-rpm-build \
-sanlock-devel \
-screen \
-scrub \
-sheepdog \
-strace \
-sudo \
-systemtap-sdt-devel \
-vim \
-wireshark-devel \
-xen-devel \
-xfsprogs-devel \
-yajl-devel \
-zfs-fuse && \
-yum autoremove -y && \
-yum clean all -y
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dockerfiles PATCH] Add git-publish configuration file

2019-05-03 Thread Andrea Bolognani 
Signed-off-by: Andrea Bolognani 
---
 .gitpublish | 4 
 1 file changed, 4 insertions(+)
 create mode 100644 .gitpublish

diff --git a/.gitpublish b/.gitpublish
new file mode 100644
index 000..a56a6d0
--- /dev/null
+++ b/.gitpublish
@@ -0,0 +1,4 @@
+[gitpublishprofile "default"]
+base = master
+to = libvir-list@redhat.com
+prefix = dockerfiles PATCH
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dockerfiles PATCH 2/3] Add Fedora 30

2019-05-03 Thread Andrea Bolognani 
The corresponding libvirt-jenkins-ci commit is b73a4dc8f6b9.

Signed-off-by: Andrea Bolognani 
---
 buildenv-fedora-30.Dockerfile | 88 +++
 1 file changed, 88 insertions(+)
 create mode 100644 buildenv-fedora-30.Dockerfile

diff --git a/buildenv-fedora-30.Dockerfile b/buildenv-fedora-30.Dockerfile
new file mode 100644
index 000..d4d5da9
--- /dev/null
+++ b/buildenv-fedora-30.Dockerfile
@@ -0,0 +1,88 @@
+FROM fedora:30
+
+RUN yum update -y && \
+yum install -y audit-libs-devel \
+augeas \
+autoconf \
+automake \
+avahi-devel \
+bash \
+bash-completion \
+ca-certificates \
+ccache \
+chrony \
+cppi \
+cyrus-sasl-devel \
+dbus-devel \
+device-mapper-devel \
+dnsmasq \
+dwarves \
+ebtables \
+fuse-devel \
+gcc \
+gdb \
+gettext \
+gettext-devel \
+git \
+glibc-devel \
+glusterfs-api-devel \
+gnutls-devel \
+iproute \
+iproute-tc \
+iscsi-initiator-utils \
+kmod \
+libacl-devel \
+libattr-devel \
+libblkid-devel \
+libcap-ng-devel \
+libcurl-devel \
+libiscsi-devel \
+libnl3-devel \
+libpcap-devel \
+libpciaccess-devel \
+librbd-devel \
+libselinux-devel \
+libssh-devel \
+libssh2-devel \
+libtirpc-devel \
+libtool \
+libudev-devel \
+libwsman-devel \
+libxml2 \
+libxml2-devel \
+libxslt \
+lsof \
+lvm2 \
+make \
+ncurses-devel \
+net-tools \
+netcf-devel \
+nfs-utils \
+numactl-devel \
+numad \
+parted \
+parted-devel \
+patch \
+perl \
+pkgconfig \
+polkit \
+qemu-img \
+radvd \
+readline-devel \
+rpcgen \
+rpm-build \
+sanlock-devel \
+screen \
+scrub \
+sheepdog \
+strace \
+sudo \
+systemtap-sdt-devel \
+vim \
+wireshark-devel \
+xen-devel \
+xfsprogs-devel \
+yajl-devel \
+zfs-fuse && \
+yum autoremove -y && \
+yum clean all -y
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dockerfiles PATCH 1/3] Refresh after adding ncurses for libvirt

2019-05-03 Thread Andrea Bolognani 
The corresponding libvirt-jenkins-ci commit is 3a79fafa0cfc.

Signed-off-by: Andrea Bolognani 
---
 buildenv-centos-7.Dockerfile  | 1 +
 buildenv-debian-9-cross-aarch64.Dockerfile| 1 +
 buildenv-debian-9-cross-armv6l.Dockerfile | 1 +
 buildenv-debian-9-cross-armv7l.Dockerfile | 1 +
 buildenv-debian-9-cross-mips.Dockerfile   | 1 +
 buildenv-debian-9-cross-mips64el.Dockerfile   | 1 +
 buildenv-debian-9-cross-mipsel.Dockerfile | 1 +
 buildenv-debian-9-cross-ppc64le.Dockerfile| 1 +
 buildenv-debian-9-cross-s390x.Dockerfile  | 1 +
 buildenv-debian-9.Dockerfile  | 1 +
 buildenv-debian-sid-cross-aarch64.Dockerfile  | 1 +
 buildenv-debian-sid-cross-armv6l.Dockerfile   | 1 +
 buildenv-debian-sid-cross-armv7l.Dockerfile   | 1 +
 buildenv-debian-sid-cross-i686.Dockerfile | 1 +
 buildenv-debian-sid-cross-mips.Dockerfile | 1 +
 buildenv-debian-sid-cross-mips64el.Dockerfile | 1 +
 buildenv-debian-sid-cross-mipsel.Dockerfile   | 1 +
 buildenv-debian-sid-cross-ppc64le.Dockerfile  | 1 +
 buildenv-debian-sid-cross-s390x.Dockerfile| 1 +
 buildenv-debian-sid.Dockerfile| 1 +
 buildenv-fedora-28.Dockerfile | 1 +
 buildenv-fedora-29.Dockerfile | 1 +
 buildenv-fedora-rawhide.Dockerfile| 1 +
 buildenv-ubuntu-18.Dockerfile | 1 +
 24 files changed, 24 insertions(+)

diff --git a/buildenv-centos-7.Dockerfile b/buildenv-centos-7.Dockerfile
index c58b563..9e79723 100644
--- a/buildenv-centos-7.Dockerfile
+++ b/buildenv-centos-7.Dockerfile
@@ -51,6 +51,7 @@ RUN yum update -y && \
 lsof \
 lvm2 \
 make \
+ncurses-devel \
 net-tools \
 netcf-devel \
 nfs-utils \
diff --git a/buildenv-debian-9-cross-aarch64.Dockerfile 
b/buildenv-debian-9-cross-aarch64.Dockerfile
index 6fc..45bbe80 100644
--- a/buildenv-debian-9-cross-aarch64.Dockerfile
+++ b/buildenv-debian-9-cross-aarch64.Dockerfile
@@ -72,6 +72,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libfuse-dev:arm64 \
 libgnutls28-dev:arm64 \
 libiscsi-dev:arm64 \
+libncurses5-dev:arm64 \
 libnl-3-dev:arm64 \
 libnl-route-3-dev:arm64 \
 libnuma-dev:arm64 \
diff --git a/buildenv-debian-9-cross-armv6l.Dockerfile 
b/buildenv-debian-9-cross-armv6l.Dockerfile
index 846f4af..4616fb2 100644
--- a/buildenv-debian-9-cross-armv6l.Dockerfile
+++ b/buildenv-debian-9-cross-armv6l.Dockerfile
@@ -72,6 +72,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libfuse-dev:armel \
 libgnutls28-dev:armel \
 libiscsi-dev:armel \
+libncurses5-dev:armel \
 libnl-3-dev:armel \
 libnl-route-3-dev:armel \
 libparted-dev:armel \
diff --git a/buildenv-debian-9-cross-armv7l.Dockerfile 
b/buildenv-debian-9-cross-armv7l.Dockerfile
index 4ab4ff4..13109cd 100644
--- a/buildenv-debian-9-cross-armv7l.Dockerfile
+++ b/buildenv-debian-9-cross-armv7l.Dockerfile
@@ -72,6 +72,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libfuse-dev:armhf \
 libgnutls28-dev:armhf \
 libiscsi-dev:armhf \
+libncurses5-dev:armhf \
 libnl-3-dev:armhf \
 libnl-route-3-dev:armhf \
 libparted-dev:armhf \
diff --git a/buildenv-debian-9-cross-mips.Dockerfile 
b/buildenv-debian-9-cross-mips.Dockerfile
index b136e43..5417d7c 100644
--- a/buildenv-debian-9-cross-mips.Dockerfile
+++ b/buildenv-debian-9-cross-mips.Dockerfile
@@ -72,6 +72,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libfuse-dev:mips \
 libgnutls28-dev:mips \
 libiscsi-dev:mips \
+libncurses5-dev:mips \
 libnl-3-dev:mips \
 libnl-route-3-dev:mips \
 libnuma-dev:mips \
diff --git a/buildenv-debian-9-cross-mips64el.Dockerfile 
b/buildenv-debian-9-cross-mips64el.Dockerfile
index c2283a4..569309c 100644
--- a/buildenv-debian-9-cross-mips64el.Dockerfile
+++ b/buildenv-debian-9-cross-mips64el.Dockerfile
@@ -72,6 +72,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libfuse-dev:mips64el \
 libgnutls28-dev:mips64el \
 libiscsi-dev:mips64el \
+libncurses5-dev:mips64el \
 libnl-3-dev:mips64el \
 libnl-route-3-dev:mips64el \
 libnuma-dev:mips64el \
diff --git a/buildenv-debian-9-cross-mipsel.Dockerfile 
b/buildenv-debian-9-cross-mipsel.Dockerfile
index f6a9044..9974776 100644
--- a/buildenv-debian-9-cross-mipsel.Dockerfile
+++ b/buildenv-debian-9-cross-mipsel.Dockerfile
@@ -72,6 +72,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libfuse-dev:mipsel \
 libgnutls28-dev:mipsel \
 libiscsi-dev:mipsel \
+libncurses5-dev:mipsel \
 libnl-3-dev:mipsel \
 libnl-route-3-dev:mip

[libvirt] [dockerfiles PATCH 0/3] Refresh, add Fedora 30, drop Fedora 28

2019-05-03 Thread Andrea Bolognani 
Pushed under the Dockerfile refresh rule.

Andrea Bolognani (3):
  Refresh after adding ncurses for libvirt
  Add Fedora 30
  Drop Fedora 28

 buildenv-centos-7.Dockerfile   | 1 +
 buildenv-debian-9-cross-aarch64.Dockerfile | 1 +
 buildenv-debian-9-cross-armv6l.Dockerfile  | 1 +
 buildenv-debian-9-cross-armv7l.Dockerfile  | 1 +
 buildenv-debian-9-cross-mips.Dockerfile| 1 +
 buildenv-debian-9-cross-mips64el.Dockerfile| 1 +
 buildenv-debian-9-cross-mipsel.Dockerfile  | 1 +
 buildenv-debian-9-cross-ppc64le.Dockerfile | 1 +
 buildenv-debian-9-cross-s390x.Dockerfile   | 1 +
 buildenv-debian-9.Dockerfile   | 1 +
 buildenv-debian-sid-cross-aarch64.Dockerfile   | 1 +
 buildenv-debian-sid-cross-armv6l.Dockerfile| 1 +
 buildenv-debian-sid-cross-armv7l.Dockerfile| 1 +
 buildenv-debian-sid-cross-i686.Dockerfile  | 1 +
 buildenv-debian-sid-cross-mips.Dockerfile  | 1 +
 buildenv-debian-sid-cross-mips64el.Dockerfile  | 1 +
 buildenv-debian-sid-cross-mipsel.Dockerfile| 1 +
 buildenv-debian-sid-cross-ppc64le.Dockerfile   | 1 +
 buildenv-debian-sid-cross-s390x.Dockerfile | 1 +
 buildenv-debian-sid.Dockerfile | 1 +
 buildenv-fedora-29.Dockerfile  | 1 +
 buildenv-fedora-28.Dockerfile => buildenv-fedora-30.Dockerfile | 3 ++-
 buildenv-fedora-rawhide.Dockerfile | 1 +
 buildenv-ubuntu-18.Dockerfile  | 1 +
 24 files changed, 25 insertions(+), 1 deletion(-)
 rename buildenv-fedora-28.Dockerfile => buildenv-fedora-30.Dockerfile (98%)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] RFC: use a slirp helper process

2019-05-03 Thread Marc-André Lureau 
Hi

On Tue, Apr 30, 2019 at 7:31 PM Daniel P. Berrangé  wrote:
> > At this point, the slirp-helper doesn't handle migration. But is it
> > really worth it? From a guest POV, it would look like packet lost or
> > disconnection. Adding migration handling is possible, to avoid a
> > disconnection event. How should it be done? via a libvirt provided
> > socket for storage? via its own file transferred by libvirt? via qemu
> > somehow (qemu wouldn't really know about the external process but
> > would copy around the migration bits?).
>
> How does migration actually work today with built-in slirp ? I

It is registered with save_state/load_state callbacks (old-style
vmstate), and save/load from the given stream.

It uses the "vmstate format" so far, although it could be a blob for
what qemu is concerned (there is no json-desc, although there could
be, but that would make libslirp even more relying on vmstate format).

But the interesting thing is that very few state is saved. Beside some
DHCP (bootp) state, only guestfwd sockets are saved. I suppose the
reason is that it's not "possible" to migrate a system socket. On
destination, UDP sockets will be handled by guest OS fine, new slirp
UDP socket will be created, and guest will mostly see some packet
lost. For TCP, in general, there isn't anything slirp can really do.
Even if slirp did attempt to save/restore TCP sockets, this will
result in stream corruption (or just disconnection) in the guest.

That's why it probably doesn't make much sense to save much of slirp
socket state beside guestfwd, and let the guest handle socket
disconnect/reconnect.

> Assume that all open network connections are lost right now ?

So guestfwd (& udp) sockets shouldn't be "lost" from guest POV. But
TCP sockets will be.

>
> What actual state does QEMU transfer for built-in slirp what
> does that achieve in practice ?
>
> Ideally we would "live migrate" all existing QEMU processes using
> built-in slirp to use the slirp helper.  Even if this doesn't
> actually do much useful stuff due to all connections being lost,
> it allows us to automatically enable external  slirp for everything
> out of the box. Requiring an opt-in config for external slirp makes
> it less compelling & will massively reduce uptake.

It's not reasonable to expect built-in slirp VM to migrate to external
slirp imho. Qemu doesn't know much about the external helper, it talks
ethernet frame with it only.

When external slirp process learns migration support, either libvirt
manages the "state" (read it, transfer it etc), or we could somehow
teach qemu to do it, ex "-savevm-external chardev", which would talk a
simple protocol to read/save the state.

Other ideas? thanks


-- 
Marc-André Lureau

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] tests: Add capabilities for QEMU 3.1.0 on s390x

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 10:46 +0200, Boris Fiuczynski wrote:
> In addition adjusting *s390x-latest.args files to qemu deprecation changes 
> made
> in commit e8c2c8bd078 (Prefer '-overcommit mem-lock' over -realtime mlock').
> 
> Signed-off-by: Boris Fiuczynski 
> ---
>  .../caps_3.1.0.s390x.replies  | 20893 
>  .../qemucapabilitiesdata/caps_3.1.0.s390x.xml |  2728 ++
>  ...othreads-virtio-scsi-ccw.s390x-latest.args | 2 +-
>  .../s390x-ccw-graphics.s390x-latest.args  | 2 +-
>  .../s390x-ccw-headless.s390x-latest.args  | 2 +-
>  .../vhost-vsock-ccw-auto.s390x-latest.args| 2 +-
>  .../vhost-vsock-ccw.s390x-latest.args | 2 +-
>  7 files changed, 23626 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemucapabilitiesdata/caps_3.1.0.s390x.replies
>  create mode 100644 tests/qemucapabilitiesdata/caps_3.1.0.s390x.xml

A few things.

First of all, while this message reached my inbox just fine, it
didn't reach the list yet because of the sheer size, which is way
over the configured limit and thus got stuck in the moderation,
needing manual intervention to get through. In the future, please
consider sending a snipped version to the mailing list and making
the full changes available somewhere, like I've done for example
with

  https://www.redhat.com/archives/libvir-list/2019-April/msg01490.html

You also mention adjusting some .args files: I hope you haven't
done so manually, but rather executed

  VIR_TEST_REGENERATE_OUTPUT=1 make check

and let it fix the output files for you. If not, TYL ;)

As for the patch itself: while it looks fine, personally I don't
feel we need to have capabilities for every single QEMU version on
every single architecture, so considering that you're adding 4.0.0
in the same series and there aren't any 3.1.0-specific tests, I
would say let's just skip this one version and add capabilitie for
4.0.0 only. Does that sound reasonable?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] virbuffer: Don't leak memory in virBufferAddBuffer

2019-05-03 Thread Andrea Bolognani 
On Fri, 2019-05-03 at 11:57 +0200, Michal Privoznik wrote:
> On 5/3/19 11:18 AM, Andrea Bolognani wrote:
> > On Thu, 2019-04-18 at 14:11 +0200, Michal Privoznik wrote:
> > > +if (virBufferCurrentContent(&buf1) ||
> > > +!virBufferCurrentContent(&buf2)) {
> > > +VIR_TEST_DEBUG("Unexpected buffer content");
> > > +goto cleanup;
> > > +}
> > 
> > And here you check both buffers. This is the part I don't get: since
> > virBufferCurrentContent() returns NULL for errored buffers, shouldn't
> > we get NULL in both cases here? And should we not get that result
> > both with and *without* your patch? We were already setting the error
> > flag for buf1 after all...
> > 
> > I tried reverting the changes to virBufferAddBuffer() made in this
> > commit, and 'make check' still passes for me, which matches my
> > observation above but certainly not my original expectations.
> 
> Firstly, it's not about passing the test but about leaking memory. Try 
> running under valgrind.

Okay, this is the first bit that I was missing, though I suspected
something along those lines. FWIW it would have been more obvious,
at least to me, what was going on if you had introduced the test
first, in a separate commit, and then fixed the function, but the
way you're doing it is just fine. I blame Friday, and not having
had much coffee in the morning :)

> Secondly, virBufferAddBuffer() resets the other buffer, so even if it 
> had an error it no longer has it after virBufferAddBuffer() is called. 
> And no error means virBufferCurrentContent() returns an empty string 
> (because the buffer has no content).

This is the second, and most important, bit I was missing: the fact
that resetting the buffer also resets the error flag. It was right
in front of my eyes, but I was not seeing it... See above again for
remarks about weekdays and beverages.

> Hopefully this clears your concerns.

It sure does! Thanks for taking the time to explain in detail.

  Reviewed-by: Andrea Bolognani 

whether or not you switch to VIR_AUTOCLEAN() - but please do :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH] lcitool: don't use SafeConfigParser under Python3

2019-05-03 Thread Daniel P . Berrangé 
Since Python 3.2 the SafeConfigParser class is renamed to
just ConfigParser. The SafeConfigParser alias is targetted
for deletion.

 ./lcitool:224: DeprecationWarning: The SafeConfigParser class
  has been renamed to ConfigParser in Python 3.2. This alias
  will be removed in future versions. Use ConfigParser directly
  instead.

This switches to use ConfigParser all the time on Py3. Technically
this is wrong on 3.0 and 3.1, but in reality no one will still be
using those, only more modern 3.x versions, or the legacy 2.7.

Signed-off-by: Daniel P. Berrangé 
---
 guests/lcitool | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index d3937be..1c18b5a 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -31,9 +31,9 @@ import yaml
 
 # This is necessary to maintain Python 2.7 compatibility
 try:
-import configparser
+from configparser import ConfigParser
 except ImportError:
-import ConfigParser as configparser
+from ConfigParser import SafeConfigParser as ConfigParser
 
 
 class Util:
@@ -221,7 +221,7 @@ class Inventory:
 ansible_cfg_path = os.path.join(base, "ansible.cfg")
 
 try:
-parser = configparser.SafeConfigParser()
+parser = ConfigParser()
 parser.read(ansible_cfg_path)
 inventory_path = parser.get("defaults", "inventory")
 except Exception as ex:
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH] lcitool: use yaml.safe_load instead of load

2019-05-03 Thread Daniel P . Berrangé 
The yaml.load() method is historically unsafe as it allowed for
arbitrary code execution:

./lcitool:323: YAMLLoadWarning: calling yaml.load() without
 Loader=... is deprecated, as the default Loader is unsafe.
 Please read https://msg.pyyaml.org/load for full details.

The PyYAML >= 5.1 is now safe by default, but has none the less
deprecated the plain load() method to avoid risk for people
running their app on older versions. For our needs safe_load()
suffices and is compatible with RHEL-7

Signed-off-by: Daniel P. Berrangé 
---
 guests/lcitool | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 1c18b5a..30b6430 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -257,7 +257,7 @@ class Inventory:
 @staticmethod
 def _add_facts_from_file(facts, yaml_path):
 with open(yaml_path, "r") as infile:
-some_facts = yaml.load(infile)
+some_facts = yaml.safe_load(infile)
 for fact in some_facts:
 facts[fact] = some_facts[fact]
 
@@ -301,7 +301,7 @@ class Projects:
 
 try:
 with open(mappings_path, "r") as infile:
-mappings = yaml.load(infile)
+mappings = yaml.safe_load(infile)
 self._mappings = mappings["mappings"]
 except Exception as ex:
 raise Exception("Can't load mappings: {}".format(ex))
@@ -320,7 +320,7 @@ class Projects:
 
 try:
 with open(yaml_path, "r") as infile:
-packages = yaml.load(infile)
+packages = yaml.safe_load(infile)
 self._packages[project] = packages["packages"]
 except Exception as ex:
 raise Exception(
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH

2019-05-03 Thread Daniel P . Berrangé 
This improves error reporting:

  $ ./lcitool install libvirt-fedora-29
  ./lcitool: Failed to install 'libvirt-fedora-29': [Errno 2] No such file or 
directory

To

  $ ./lcitool install libvirt-fedora-29
  ./lcitool: Cannot find virt-install in $PATH

Signed-off-by: Daniel P. Berrangé 
---
 guests/lcitool | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 0f60704..d3937be 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -17,6 +17,7 @@
 # with this program. If not, see .
 
 import argparse
+import distutils.spawn
 import fnmatch
 import json
 import os
@@ -461,8 +462,12 @@ class Application:
 "git_branch": git_branch,
 })
 
+ap_path = distutils.spawn.find_executable("ansible-playbook")
+if ap_path is None:
+raise Exception("Cannot find ansible-playbook in $PATH")
+
 cmd = [
-"ansible-playbook",
+ap_path,
 "--limit", ansible_hosts,
 "--extra-vars", extra_vars,
 ]
@@ -534,8 +539,12 @@ class Application:
 # a kernel argument
 extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
 
+vi_path = distutils.spawn.find_executable("virt-install")
+if vi_path is None:
+raise Exception("Cannot find virt-install in $PATH")
+
 cmd = [
-"virt-install",
+vi_path,
 "--name", host,
 "--location", facts["install_url"],
 "--virt-type", facts["install_virt_type"],
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] virbuffer: Don't leak memory in virBufferAddBuffer

2019-05-03 Thread Michal Privoznik 

On 5/3/19 11:18 AM, Andrea Bolognani wrote:

On Thu, 2019-04-18 at 14:11 +0200, Michal Privoznik wrote:

If an error occurs in a virBuffer* API the idea is to free the
content immediately and set @error member used in error reporting
later. Well, this is not what how virBufferAddBuffer works.

Signed-off-by: Michal Privoznik 
---
  src/util/virbuffer.c |  2 +-
  tests/virbuftest.c   | 32 
  2 files changed, 33 insertions(+), 1 deletion(-)


I'm a bit confused, so I'll spell everything out and you can tell me
what I'm getting wrong :)


@@ -197,7 +197,7 @@ virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd)
  
  if (buf->error || toadd->error) {

  if (!buf->error)
-buf->error = toadd->error;
+virBufferSetError(buf, toadd->error);
  goto done;
  }


This change makes sense to me: instead of simply setting the error
flag in the destination buffer, we use virBufferSetError() so that
we will set the error flag *and* also free all memory associated
with the buffer. This is consistent with the commit message and the
comments in the newly-added test case, where you claim you're
addressing a memory leak. So far, so good.

[...]

+static int
+testBufAddBuffer2(const void *opaque ATTRIBUTE_UNUSED)
+{
+virBuffer buf1 = VIR_BUFFER_INITIALIZER;
+virBuffer buf2 = VIR_BUFFER_INITIALIZER;
+int ret = -1;
+
+/* Intent of this test is to demonstrate a memleak that happen with
+ * virBufferAddBuffer */
+
+virBufferAddLit(&buf1, "Hello world!\n");
+virBufferAddLit(&buf2, "Hello world!\n");


Here you're adding some data to the buffers. They're both in a sane
state for the time being.


+/* Intentional usage error */
+virBufferAdjustIndent(&buf2, -2);


Here you reduce by 2 the indentation of buf2; however, since the
indentation for the buffer was 0 before the call, this is not a
valid use of the API: the memory allocated to buf2 is released, and
the error flag for it is set.


Yes, as the comment says, this is intentional. The idea is to make the 
very next call fail.





+virBufferAddBuffer(&buf1, &buf2);


Now you try to add the (errored) buf2 to buf1, which should result
in buf1 being unallocated and put into an error state as well. So
at this point the memory allocated to both buffers should have been
released, and the error flag should have been set for both.


+if (virBufferCurrentContent(&buf1) ||
+!virBufferCurrentContent(&buf2)) {
+VIR_TEST_DEBUG("Unexpected buffer content");
+goto cleanup;
+}


And here you check both buffers. This is the part I don't get: since
virBufferCurrentContent() returns NULL for errored buffers, shouldn't
we get NULL in both cases here? And should we not get that result
both with and *without* your patch? We were already setting the error
flag for buf1 after all...

I tried reverting the changes to virBufferAddBuffer() made in this
commit, and 'make check' still passes for me, which matches my
observation above but certainly not my original expectations.


Firstly, it's not about passing the test but about leaking memory. Try 
running under valgrind.
Secondly, virBufferAddBuffer() resets the other buffer, so even if it 
had an error it no longer has it after virBufferAddBuffer() is called. 
And no error means virBufferCurrentContent() returns an empty string 
(because the buffer has no content).


Hopefully this clears your concerns.




+ret = 0;
+ cleanup:
+virBufferFreeAndReset(&buf1);
+virBufferFreeAndReset(&buf2);
+return ret;


Stray observation: you can use VIR_AUTOCLEAR() when declaring the
buffers and drop the cleanup path entirely.



Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] virbuffer: Don't leak memory in virBufferAddBuffer

2019-05-03 Thread Andrea Bolognani 
On Thu, 2019-04-18 at 14:11 +0200, Michal Privoznik wrote:
> If an error occurs in a virBuffer* API the idea is to free the
> content immediately and set @error member used in error reporting
> later. Well, this is not what how virBufferAddBuffer works.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virbuffer.c |  2 +-
>  tests/virbuftest.c   | 32 
>  2 files changed, 33 insertions(+), 1 deletion(-)

I'm a bit confused, so I'll spell everything out and you can tell me
what I'm getting wrong :)

> @@ -197,7 +197,7 @@ virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd)
>  
>  if (buf->error || toadd->error) {
>  if (!buf->error)
> -buf->error = toadd->error;
> +virBufferSetError(buf, toadd->error);
>  goto done;
>  }

This change makes sense to me: instead of simply setting the error
flag in the destination buffer, we use virBufferSetError() so that
we will set the error flag *and* also free all memory associated
with the buffer. This is consistent with the commit message and the
comments in the newly-added test case, where you claim you're
addressing a memory leak. So far, so good.

[...]
> +static int
> +testBufAddBuffer2(const void *opaque ATTRIBUTE_UNUSED)
> +{
> +virBuffer buf1 = VIR_BUFFER_INITIALIZER;
> +virBuffer buf2 = VIR_BUFFER_INITIALIZER;
> +int ret = -1;
> +
> +/* Intent of this test is to demonstrate a memleak that happen with
> + * virBufferAddBuffer */
> +
> +virBufferAddLit(&buf1, "Hello world!\n");
> +virBufferAddLit(&buf2, "Hello world!\n");

Here you're adding some data to the buffers. They're both in a sane
state for the time being.

> +/* Intentional usage error */
> +virBufferAdjustIndent(&buf2, -2);

Here you reduce by 2 the indentation of buf2; however, since the
indentation for the buffer was 0 before the call, this is not a
valid use of the API: the memory allocated to buf2 is released, and
the error flag for it is set.

> +virBufferAddBuffer(&buf1, &buf2);

Now you try to add the (errored) buf2 to buf1, which should result
in buf1 being unallocated and put into an error state as well. So
at this point the memory allocated to both buffers should have been
released, and the error flag should have been set for both.

> +if (virBufferCurrentContent(&buf1) ||
> +!virBufferCurrentContent(&buf2)) {
> +VIR_TEST_DEBUG("Unexpected buffer content");
> +goto cleanup;
> +}

And here you check both buffers. This is the part I don't get: since
virBufferCurrentContent() returns NULL for errored buffers, shouldn't
we get NULL in both cases here? And should we not get that result
both with and *without* your patch? We were already setting the error
flag for buf1 after all...

I tried reverting the changes to virBufferAddBuffer() made in this
commit, and 'make check' still passes for me, which matches my
observation above but certainly not my original expectations.

> +ret = 0;
> + cleanup:
> +virBufferFreeAndReset(&buf1);
> +virBufferFreeAndReset(&buf2);
> +return ret;

Stray observation: you can use VIR_AUTOCLEAR() when declaring the
buffers and drop the cleanup path entirely.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] virbuffer: Use signed integer for storing error

2019-05-03 Thread Michal Privoznik 

On 5/3/19 10:30 AM, Erik Skultety wrote:

On Thu, Apr 18, 2019 at 02:11:24PM +0200, Michal Privoznik wrote:

The @error member can contain a positive value (errno) or a
negative value (-1) to denote an usage error. It doesn't make


s/an usage/a usage

--> even though there are 2 vowels in a row, pronounciation is important, since
you pronounce it as [ˈjuːzɪʤ], you need 'a' rather than 'an'. An opposite
example would be 'a hour' vs 'an hour' where the latter is correct because of
the vowel sound in pronouncing 'hour'.


https://en.wikipedia.org/wiki/Ghoti




much sense to store it as unsigned then.

Signed-off-by: Michal Privoznik 
---

trivial

Reviewed-by: Erik Skultety 



Thanks,
Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] virBuffer: Try harder to free buffer

2019-05-03 Thread Michal Privoznik 

On 5/3/19 11:01 AM, Erik Skultety wrote:

On Thu, Apr 18, 2019 at 02:11:25PM +0200, Michal Privoznik wrote:

Currently, the way virBufferFreeAndReset() works is it relies on
virBufferContentAndReset() to fetch the buffer content which is
then freed. This works as long as there is no bug in virBuffer*
implementation (not true apparently). Explicitly call free() over
buffer content.

Signed-off-by: Michal Privoznik 
---
  src/util/virbuffer.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index b2ae7963a1..ac03b15a61 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -281,9 +281,8 @@ virBufferContentAndReset(virBufferPtr buf)
   */
  void virBufferFreeAndReset(virBufferPtr buf)
  {
-char *str = virBufferContentAndReset(buf);
-
-VIR_FREE(str);
+if (buf)
+virBufferSetError(buf, 0);


You saved 1 call to memset. I also don't see how we can break
virBufferContentAndReset in a way that we couldn't mess up virBufferSetError in
the same way. Additionally, seeing a call to virBufferSetError in a free helper
looks suspicious (even if it has a '0' as argument). If anything, I'd rename
virBuggerFreeAndReset to virBufferClean.


Thing is, if 1/3 wasn't merged, then the test I'm introducing there 
would leak memory. But not with this patch merged. So from long term 
perspective, if there is ever some similar bug we won't leak any memory.


Yeah, naming is hard.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] virbuffer: Don't leak memory in virBufferAddBuffer

2019-05-03 Thread Michal Privoznik 

On 5/3/19 10:40 AM, Erik Skultety wrote:

On Thu, Apr 18, 2019 at 02:11:23PM +0200, Michal Privoznik wrote:

If an error occurs in a virBuffer* API the idea is to free the
content immediately and set @error member used in error reporting
later. Well, this is not what how virBufferAddBuffer works.

Signed-off-by: Michal Privoznik 
---
  src/util/virbuffer.c |  2 +-
  tests/virbuftest.c   | 32 
  2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 54703a28d8..b2ae7963a1 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -197,7 +197,7 @@ virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd)

  if (buf->error || toadd->error) {
  if (!buf->error)
-buf->error = toadd->error;
+virBufferSetError(buf, toadd->error);
  goto done;


This indeed fixes the problem. But looking at the test below it got me
wondering, why we actually define the functions as void and free the original
buffer on error with setting a numerical error. We usually leave the cleanup to
the caller if something goes wrong. Failing to add to a buffer is IMHO quite a
serious problem, because it can be either allocation problem (in which case
there are much  bigger problems) or an internal problem which also cannot be
ignored because we store data in there so it opens a door to not only
corruption but also inconsistency.



The idea of virBuffer is to just write all the calls and only then check 
for an error. For instance like this:


virBuffer buf = VIR_BUFFER_INITIALIZER;
virBuffer buf2 = VIR_BUFFER_INITIALIZER;

virBufferAsprintf(&buf, "Hello world!");
virBufferAddLit(&buf, "\n");

virBufferAsprintf(&buf2, "Child reported:\n");
virBufferAddBuffer(&buf2, &buf);

virBufferChecError(&buf2);

We just call virBuffer*() and only at the end, just before we'd get its 
content check for an error. Note, that any of those calls can fail (e.g. 
due to OOM) but they're still declared as void.


Failing to add a buffer is not different to adding just any other string 
really.




What I'd expect is that if the operation fails, we return -1, caller decides
for themself whether they want to ignore or act upon it; the data is inherently
invalidated, but they still should be able to cleanup the mess. Optionally, we
could go one safety step further and not touch caller provided data to be
modified until the last moment where we just swap pointers and thus the
original isn't corrupted (we also do this at many places, but I understand
"nice-to-have's" bring a certain burden and potential issues on their own).


Not really. What difference does it make if the error is checked for 
later? If there's an error, it is stored inside the virBuffer struct and 
all subsequent virBuffer*() calls turn to NOPs. Except 
virBufferCheckError() which fetches the recorded error.




To the patch:
Reviewed-by: Erik Skultety 


Thanks,
Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] virBuffer: Try harder to free buffer

2019-05-03 Thread Erik Skultety 
On Thu, Apr 18, 2019 at 02:11:25PM +0200, Michal Privoznik wrote:
> Currently, the way virBufferFreeAndReset() works is it relies on
> virBufferContentAndReset() to fetch the buffer content which is
> then freed. This works as long as there is no bug in virBuffer*
> implementation (not true apparently). Explicitly call free() over
> buffer content.
>
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virbuffer.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
> index b2ae7963a1..ac03b15a61 100644
> --- a/src/util/virbuffer.c
> +++ b/src/util/virbuffer.c
> @@ -281,9 +281,8 @@ virBufferContentAndReset(virBufferPtr buf)
>   */
>  void virBufferFreeAndReset(virBufferPtr buf)
>  {
> -char *str = virBufferContentAndReset(buf);
> -
> -VIR_FREE(str);
> +if (buf)
> +virBufferSetError(buf, 0);

You saved 1 call to memset. I also don't see how we can break
virBufferContentAndReset in a way that we couldn't mess up virBufferSetError in
the same way. Additionally, seeing a call to virBufferSetError in a free helper
looks suspicious (even if it has a '0' as argument). If anything, I'd rename
virBuggerFreeAndReset to virBufferClean.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/4] tests: domaincaps: Add QEMU 4.0.0 for s390x

2019-05-03 Thread Boris Fiuczynski 
Signed-off-by: Boris Fiuczynski 
---
 .../domaincapsschemadata/qemu_4.0.0.s390x.xml | 198 ++
 tests/domaincapstest.c|   4 +
 2 files changed, 202 insertions(+)
 create mode 100644 tests/domaincapsschemadata/qemu_4.0.0.s390x.xml

diff --git a/tests/domaincapsschemadata/qemu_4.0.0.s390x.xml 
b/tests/domaincapsschemadata/qemu_4.0.0.s390x.xml
new file mode 100644
index 00..edade48ad0
--- /dev/null
+++ b/tests/domaincapsschemadata/qemu_4.0.0.s390x.xml
@@ -0,0 +1,198 @@
+
+  /usr/bin/qemu-system-s390x
+  kvm
+  s390-ccw-virtio-4.0
+  s390x
+  
+  
+  
+
+
+  /usr/share/AAVMF/AAVMF_CODE.fd
+  /usr/share/AAVMF/AAVMF32_CODE.fd
+  /usr/share/OVMF/OVMF_CODE.fd
+  
+rom
+pflash
+  
+  
+yes
+no
+  
+  
+no
+  
+
+  
+  
+
+
+  z14.2-base
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+  z14.2
+  z890.2
+  z990.4
+  z14ZR1
+  z10BC.2
+  z196.2
+  z14
+  z9BC-base
+  zEC12-base
+  z196-base
+  z13-base
+  z990.3
+  z9EC
+  zBC12
+  z9EC.3
+  z196.2-base
+  qemu
+  zEC12.2-base
+  z800-base
+  z9EC.2
+  z900.2-base
+  z14ZR1-base
+  z900.3
+  z890
+  z890-base
+  z990.4-base
+  z10BC.2-base
+  z900.2
+  z9BC.2-base
+  z800
+  z114
+  z13
+  z990
+  z13s-base
+  z990.2
+  z14.2-base
+  z14-base
+  z890.2-base
+  z196
+  z10EC
+  z13s
+  z900
+  z10EC.3
+  z10EC.2-base
+  z114-base
+  z990.2-base
+  z9EC.2-base
+  z890.3
+  z900.3-base
+  z9BC.2
+  z10BC
+  z990.5
+  zEC12.2
+  z10EC-base
+  z9EC-base
+  z9EC.3-base
+  zEC12
+  z990.5-base
+  z10BC-base
+  max
+  z900-base
+  z13.2
+  zBC12-base
+  z13.2-base
+  z890.3-base
+  z990-base
+  z10EC.2
+  z9BC
+  z10EC.3-base
+  z990.3-base
+
+  
+  
+
+  
+disk
+cdrom
+floppy
+lun
+  
+  
+fdc
+scsi
+virtio
+  
+  
+virtio
+virtio-transitional
+virtio-non-transitional
+  
+
+
+  
+sdl
+vnc
+  
+
+
+  
+virtio
+  
+
+
+  
+subsystem
+  
+  
+default
+mandatory
+requisite
+optional
+  
+  
+usb
+pci
+scsi
+  
+  
+  
+default
+kvm
+vfio
+  
+
+  
+  
+
+
+
+
+  
+
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 01bb402c2d..98ca2531d8 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -453,6 +453,10 @@ mymain(void)
 DO_TEST_QEMU("4.0.0", "caps_4.0.0",
  "/usr/bin/qemu-system-x86_64", NULL,
  "x86_64", VIR_DOMAIN_VIRT_KVM);
+
+DO_TEST_QEMU("4.0.0", "caps_4.0.0",
+ "/usr/bin/qemu-system-s390x", NULL,
+ "s390x", VIR_DOMAIN_VIRT_KVM);
 virObjectUnref(cfg);
 
 virFileWrapperClearPrefixes();
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/4] tests: domaincaps: Add QEMU 3.1.0 for s390x

2019-05-03 Thread Boris Fiuczynski 
Signed-off-by: Boris Fiuczynski 
---
 .../domaincapsschemadata/qemu_3.1.0.s390x.xml | 196 ++
 tests/domaincapstest.c|   4 +
 2 files changed, 200 insertions(+)
 create mode 100644 tests/domaincapsschemadata/qemu_3.1.0.s390x.xml

diff --git a/tests/domaincapsschemadata/qemu_3.1.0.s390x.xml 
b/tests/domaincapsschemadata/qemu_3.1.0.s390x.xml
new file mode 100644
index 00..2a7171b22f
--- /dev/null
+++ b/tests/domaincapsschemadata/qemu_3.1.0.s390x.xml
@@ -0,0 +1,196 @@
+
+  /usr/bin/qemu-system-s390x
+  kvm
+  s390-ccw-virtio-3.1
+  s390x
+  
+  
+  
+
+
+  /usr/share/AAVMF/AAVMF_CODE.fd
+  /usr/share/AAVMF/AAVMF32_CODE.fd
+  /usr/share/OVMF/OVMF_CODE.fd
+  
+rom
+pflash
+  
+  
+yes
+no
+  
+  
+no
+  
+
+  
+  
+
+
+  z14-base
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+  z890.2
+  z990.4
+  z14ZR1
+  z10BC.2
+  z196.2
+  z14
+  z9BC-base
+  zEC12-base
+  z196-base
+  z13-base
+  z990.3
+  z9EC
+  zBC12
+  z9EC.3
+  z196.2-base
+  qemu
+  zEC12.2-base
+  z800-base
+  z9EC.2
+  z900.2-base
+  z14ZR1-base
+  z900.3
+  z890-base
+  z890
+  z990.4-base
+  z10BC.2-base
+  z900.2
+  z9BC.2-base
+  z800
+  z114
+  z13
+  z13s-base
+  z990
+  z990.2
+  z14-base
+  z890.2-base
+  z196
+  z10EC
+  z13s
+  z900
+  z10EC.3
+  z10EC.2-base
+  z114-base
+  z990.2-base
+  z9EC.2-base
+  z890.3
+  z900.3-base
+  z9BC.2
+  z10BC
+  z990.5
+  zEC12.2
+  z10EC-base
+  z9EC-base
+  z9EC.3-base
+  zEC12
+  z990.5-base
+  z10BC-base
+  max
+  z900-base
+  z13.2
+  zBC12-base
+  z13.2-base
+  z890.3-base
+  z990-base
+  z10EC.2
+  z9BC
+  z10EC.3-base
+  z990.3-base
+
+  
+  
+
+  
+disk
+cdrom
+floppy
+lun
+  
+  
+fdc
+scsi
+virtio
+  
+  
+virtio
+virtio-transitional
+virtio-non-transitional
+  
+
+
+  
+sdl
+vnc
+  
+
+
+  
+virtio
+  
+
+
+  
+subsystem
+  
+  
+default
+mandatory
+requisite
+optional
+  
+  
+usb
+pci
+scsi
+  
+  
+  
+default
+kvm
+vfio
+  
+
+  
+  
+
+
+
+
+  
+
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index f87a1d63fb..01bb402c2d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -446,6 +446,10 @@ mymain(void)
  "/usr/bin/qemu-system-x86_64", NULL,
  "x86_64", VIR_DOMAIN_VIRT_KVM);
 
+DO_TEST_QEMU("3.1.0", "caps_3.1.0",
+ "/usr/bin/qemu-system-s390x", NULL,
+ "s390x", VIR_DOMAIN_VIRT_KVM);
+
 DO_TEST_QEMU("4.0.0", "caps_4.0.0",
  "/usr/bin/qemu-system-x86_64", NULL,
  "x86_64", VIR_DOMAIN_VIRT_KVM);
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/4] tests: Add capabilities for s390x

2019-05-03 Thread Boris Fiuczynski 
This series adds the capabilities of QEMU 3.1.0 and 4.0.0 on s390x.

Boris Fiuczynski (4):
  tests: Add capabilities for QEMU 3.1.0 on s390x
  tests: domaincaps: Add QEMU 3.1.0 for s390x
  tests: Add capabilities for QEMU 4.0.0 on s390x
  tests: domaincaps: Add QEMU 4.0.0 for s390x

 .../domaincapsschemadata/qemu_3.1.0.s390x.xml |   196 +
 .../domaincapsschemadata/qemu_4.0.0.s390x.xml |   198 +
 tests/domaincapstest.c| 8 +
 .../caps_3.1.0.s390x.replies  | 20893 +++
 .../qemucapabilitiesdata/caps_3.1.0.s390x.xml |  2728 ++
 .../caps_4.0.0.s390x.replies  | 21514 
 .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  2894 +++
 ...othreads-virtio-scsi-ccw.s390x-latest.args | 6 +-
 .../s390x-ccw-graphics.s390x-latest.args  | 2 +-
 .../s390x-ccw-headless.s390x-latest.args  | 2 +-
 .../vhost-vsock-ccw-auto.s390x-latest.args| 2 +-
 .../vhost-vsock-ccw.s390x-latest.args | 2 +-
 12 files changed, 48438 insertions(+), 7 deletions(-)
 create mode 100644 tests/domaincapsschemadata/qemu_3.1.0.s390x.xml
 create mode 100644 tests/domaincapsschemadata/qemu_4.0.0.s390x.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_3.1.0.s390x.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_3.1.0.s390x.xml
 create mode 100644 tests/qemucapabilitiesdata/caps_4.0.0.s390x.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml

-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] virbuffer: Don't leak memory in virBufferAddBuffer

2019-05-03 Thread Erik Skultety 
On Thu, Apr 18, 2019 at 02:11:23PM +0200, Michal Privoznik wrote:
> If an error occurs in a virBuffer* API the idea is to free the
> content immediately and set @error member used in error reporting
> later. Well, this is not what how virBufferAddBuffer works.
>
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virbuffer.c |  2 +-
>  tests/virbuftest.c   | 32 
>  2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
> index 54703a28d8..b2ae7963a1 100644
> --- a/src/util/virbuffer.c
> +++ b/src/util/virbuffer.c
> @@ -197,7 +197,7 @@ virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd)
>
>  if (buf->error || toadd->error) {
>  if (!buf->error)
> -buf->error = toadd->error;
> +virBufferSetError(buf, toadd->error);
>  goto done;

This indeed fixes the problem. But looking at the test below it got me
wondering, why we actually define the functions as void and free the original
buffer on error with setting a numerical error. We usually leave the cleanup to
the caller if something goes wrong. Failing to add to a buffer is IMHO quite a
serious problem, because it can be either allocation problem (in which case
there are much  bigger problems) or an internal problem which also cannot be
ignored because we store data in there so it opens a door to not only
corruption but also inconsistency.

What I'd expect is that if the operation fails, we return -1, caller decides
for themself whether they want to ignore or act upon it; the data is inherently
invalidated, but they still should be able to cleanup the mess. Optionally, we
could go one safety step further and not touch caller provided data to be
modified until the last moment where we just swap pointers and thus the
original isn't corrupted (we also do this at many places, but I understand
"nice-to-have's" bring a certain burden and potential issues on their own).

To the patch:
Reviewed-by: Erik Skultety 

But I'd like to get an opinion on the above, because I'm curious.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 1/1] tests/virhostdevtest: remove virHostdevHostSupportsPassthroughKVM

2019-05-03 Thread Michal Privoznik 

On 5/2/19 2:51 PM, Daniel Henrique Barboza wrote:

virhostdevtest is using pci mock to emulate all PCI attach/detach
operations. This means that that this test does not rely on KVM
support of the host anymore and the tests in this file shouldn't
be affected by it.

Suggested-by: Michal Privoznik 
Signed-off-by: Daniel Henrique Barboza 
---
  tests/virhostdevtest.c | 61 +-
  1 file changed, 12 insertions(+), 49 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 4e067b10d1..20eaca82e0 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -126,37 +126,6 @@ myInit(void)
  return -1;
  }
  
-# if HAVE_LINUX_KVM_H

-#  include 
-static bool
-virHostdevHostSupportsPassthroughKVM(void)
-{
-int kvmfd = -1;
-bool ret = false;
-
-if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0)
-goto cleanup;
-
-#  ifdef KVM_CAP_IOMMU
-if ((ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_IOMMU)) <= 0)
-goto cleanup;
-
-ret = true;
-#  endif
-
- cleanup:
-VIR_FORCE_CLOSE(kvmfd);
-
-return ret;
-}
-# else
-static bool
-virHostdevHostSupportsPassthroughKVM(void)
-{
-return false;
-}
-# endif
-
  static int
  testVirHostdevPreparePCIHostdevs_unmanaged(void)
  {
@@ -483,12 +452,10 @@ testVirHostdevRoundtripUnmanaged(const void *opaque 
ATTRIBUTE_UNUSED)
  
  if (testVirHostdevDetachPCINodeDevice() < 0)

  goto out;
-if (virHostdevHostSupportsPassthroughKVM()) {
-if (testVirHostdevPreparePCIHostdevs_unmanaged() < 0)
-goto out;
-if (testVirHostdevReAttachPCIHostdevs_unmanaged() < 0)
-goto out;
-}
+if (testVirHostdevPreparePCIHostdevs_unmanaged() < 0)
+goto out;
+if (testVirHostdevReAttachPCIHostdevs_unmanaged() < 0)
+goto out;
  if (testVirHostdevReAttachPCINodeDevice() < 0)
  goto out;


Huh, these branches were never run because as of v4.12-rc1~68^2~65 
kernel commit the KVM_CAP_IOMMU is dropped. And looks like RHEL-7.6 
backported the patch too (don't have anything older to test this on).


This test could also use some cleanup. I'll post patches shortly.

ACK to this one though. Will push it once the release is done.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] virbuffer: Use signed integer for storing error

2019-05-03 Thread Erik Skultety 
On Thu, Apr 18, 2019 at 02:11:24PM +0200, Michal Privoznik wrote:
> The @error member can contain a positive value (errno) or a
> negative value (-1) to denote an usage error. It doesn't make

s/an usage/a usage

--> even though there are 2 vowels in a row, pronounciation is important, since
you pronounce it as [ˈjuːzɪʤ], you need 'a' rather than 'an'. An opposite
example would be 'a hour' vs 'an hour' where the latter is correct because of
the vowel sound in pronouncing 'hour'.

> much sense to store it as unsigned then.
>
> Signed-off-by: Michal Privoznik 
> ---
trivial

Reviewed-by: Erik Skultety 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemuConnectOpen: Drop unused @cfg and simplify

2019-05-03 Thread Andrea Bolognani 
On Tue, 2019-04-30 at 11:47 +0200, Michal Privoznik wrote:
> After 65a372d6e0 the @cfg variable is no longer used. This means
> we can drop it and therefore drop 'cleanup' label with it.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_driver.c | 17 +
>  1 file changed, 5 insertions(+), 12 deletions(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list