[libvirt] Request for review: [PATCH v3 0/3] implement migrate-getmaxdowntime command

[Scott Garfinkle]

I think this may have gotten lost in the 3.6 release. Does someone have 
the time to review this, please?


The patches still apply cleanly (albeit with fuzz) at the current level.

-Scott Garfinkle


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

[libvirt] ANNOUNCE: Oz 0.16.0 release

[Chris Lalancette]

All,
I'm pleased to announce release 0.16.0 of Oz.  Oz is a program for
doing automated installation of guest operating systems with limited input
from the user.  Release 0.16.0 is a bugfix and feature release for Oz.
Some of the highlights between Oz 0.15.0 and 0.16.0 are:

* Windows 10 and 2016 support
* All timeouts are now configurable
* Ubuntu 16.04, 16.10, 17.04 support
* Mageia 2, 3, 4, 5 support
* Properly find UEFI firmware, which should fix aarch64 installs
* Fedora 24, 25, 26 support
* FreeBSD 11 support
* Replace internal use of pycurl with requests
* OpenSUSE Leap support
* Timeouts are now based on time, not the number of iterations of the loop
* Modern Fedora and RHEL guests will print out a lot more anaconda
debugging information to the terminal that oz-install was launched from

A tarball and zipfile of this release is available on the Github releases
page: https://github.com/clalancette/oz/releases .  Packages for Fedora
rawhide and 26 have been built in Koji and will eventually make their way
to stable.  Instructions on how to get and use Oz are available at
http://github.com/clalancette/oz/wiki .

If you have questions or comments about Oz, please feel free to contact me
at clalancette at gmail.com, or open up an issue on the github page:
http://github.com/clalancette/oz/issues .

Thanks to everyone who contributed to this release through bug reports,
patches, and suggestions for improvement.

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

Re: [libvirt] [RESEND] pci: add support for VMD domains

[Laine Stump]

On 08/07/2017 03:46 PM, Jon Derrick wrote:
> VMD domains start at 0x1, so expand dev->name to fit at least this > many 
> characters. > > Signed-off-by: Jon Derrick
 > --- > src/util/virpci.c | 2 +- > 1 file
changed, 1 insertion(+), 1 deletion(-) > > diff --git
a/src/util/virpci.c b/src/util/virpci.c > index 2c1b758..b3afefe 100644
> --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -50,7 +50,7 @@
VIR_LOG_INIT("util.pci"); > > #define PCI_SYSFS "/sys/bus/pci/" >
#define PCI_ID_LEN 10 /* " " */ > -#define PCI_ADDR_LEN 13 /*
":XX:XX.X" */ > +#define PCI_ADDR_LEN 14 /* "X:XX:XX.X" */ > >
VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST, > "", "2.5",
"5", "8", "16")
Does just this change by itself enable new functionality? Or are other
changes required? (e.g. the type "pciDomain" in the XML schema is a
uint16, so any domain > 0x in the config would fail validation).

Assuming that the VMD domain needs to be referenced in config somewhere
in order to be useful, along with changing the type for pciDomain in
docs/schemes/basictypes.rng, we would also need at least one new test
case for the qemuxml2argv and qemuxml2xml tests (see the examples in the
"hostdev-vfio-multidomain" and "net-hostdev-multidomain").

Also, do all versions of qemu support domains > 0x? If not, is there
a feature that can be used to detect that support so we can have a
capability bit for it and warn if someone tries to use such a domain
when the installed version of qemu doesn't support it? (If there is no
way to tell in advance, then we'll just have to live with reporting any
qemu error after the fact)

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

[libvirt] A bug that libvirt lxc can't destroy all container process.

[曹远志]

Hello all, 
This is my first mail to this list, so let me introduce myself. My name is 
yuanzhi cao, and I work in the oVirt team. Currently We are using libvirt lxc 
in large-scale and found that libvirt can't destroy container process normally .

Environments:

 System: zesty
 libvirt version: 2.5.0-3ubuntu5
 vm rootfs release: ubuntu:16.04
 
Reproduce:
 1. Run command "virsh -c lxc:// start vm" and the release of vm is xenial
 2. Run command "pa aux|grep init" ,you would find the pid of init launch by vm.
 3. Run command "virsh -c lxc:// destroy vm".
 4. Run command "virsh -c lxc:// list --all" and "ps aux|grep init" ,you  could 
find that vm is shutoff, but the init process launch by vm is  still running.
 
Infact I have found the case of this bug, there is a patch after 1.3.1 that 
import this bug.
 
-
 Commit: dc576025c360a1d2c89da410d0f3f0da55d0143f [dc57602]
 Parents: 511e7c5bba
 Author: Daniel P. Berrange 
 Date: 2016年1月23日 GMT+8 上午12:07:18
 Commit Date: 2016年1月27日 GMT+8 上午12:11:32
 lxc: don't try to hide parent cgroups inside container
 -
 
Cgroups inside container does't hide parent, so the process of container can 
change it own cgroup to  another cgroup.
 lxc destroy process by read cgroup tasks file,if process change it own 
cgroup,it can't destroy container process normally.--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH 7/7] data: add system dbus service policy configuration

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 configure.ac | 10 ++
 data/Makefile.am |  8 +++-
 data/system/org.libvirt.conf | 12 
 libvirt-dbus.spec.in |  1 +
 4 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 data/system/org.libvirt.conf

diff --git a/configure.ac b/configure.ac
index 65071f9..4c654fa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -55,6 +55,16 @@ else
 fi
 AC_SUBST(DBUS_SYSTEM_SERVICES_DIR)
 
+AC_ARG_WITH(dbus-system-policies,
+[AC_HELP_STRING([--with-dbus-system-policies=],
+[where D-BUS system policies directory is])])
+if ! test -z "$with_dbus_system_policies" ; then
+DBUS_SYSTEM_POLICIES_DIR="$with_dbus_system_policies"
+else
+DBUS_SYSTEM_POLICIES_DIR="$datadir/dbus-1/system.d"
+fi
+AC_SUBST(DBUS_SYSTEM_POLICIES_DIR)
+
 AC_OUTPUT(Makefile
   data/Makefile
   src/Makefile
diff --git a/data/Makefile.am b/data/Makefile.am
index b8f1376..58e855f 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -8,9 +8,15 @@ system_service_in_files = \
 system_servicedir = $(DBUS_SYSTEM_SERVICES_DIR)
 system_service_DATA = $(system_service_in_files:.service.in=.service)
 
+system_policy_files = \
+   system/org.libvirt.conf
+system_policydir = $(DBUS_SYSTEM_POLICIES_DIR)
+system_policy_DATA = $(system_policy_files)
+
 EXTRA_DIST = \
$(service_in_files) \
-   $(system_service_in_files)
+   $(system_service_in_files) \
+   $(system_policy_files)
 
 CLEANFILES = \
$(service_DATA) \
diff --git a/data/system/org.libvirt.conf b/data/system/org.libvirt.conf
new file mode 100644
index 000..5cbc732
--- /dev/null
+++ b/data/system/org.libvirt.conf
@@ -0,0 +1,12 @@
+
+http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd;>
+
+
+
+  
+
+
+  
+
+
diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in
index 0f76de2..5be4c22 100644
--- a/libvirt-dbus.spec.in
+++ b/libvirt-dbus.spec.in
@@ -43,5 +43,6 @@ rm -rf $RPM_BUILD_ROOT
 %{_bindir}/libvirt-dbus
 %{_datadir}/dbus-1/services/org.libvirt.service
 %{_datadir}/dbus-1/system-services/org.libvirt.service
+%{_datadir}/dbus-1/system.d/org.libvirt.conf
 
 %changelog
-- 
2.13.4

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

[libvirt] [dbus PATCH 6/7] data: add system dbus service file

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 .gitignore |  1 +
 configure.ac   | 10 ++
 data/Makefile.am   | 16 ++--
 data/system/org.libvirt.service.in |  4 
 libvirt-dbus.spec.in   |  1 +
 5 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 data/system/org.libvirt.service.in

diff --git a/.gitignore b/.gitignore
index f223068..1d41949 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,6 +23,7 @@ vgcore.*
 /stamp-h1
 
 /data/session/org.libvirt.service
+/data/system/org.libvirt.service
 
 /src/.deps/
 /src/libvirt-dbus
diff --git a/configure.ac b/configure.ac
index 158c5e9..65071f9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -45,6 +45,16 @@ else
 fi
 AC_SUBST(DBUS_SERVICES_DIR)
 
+AC_ARG_WITH(dbus-system-services,
+[AC_HELP_STRING([--with-dbus-system-services=],
+[where D-BUS system services directory is])])
+if ! test -z "$with_dbus_system_services" ; then
+DBUS_SYSTEM_SERVICES_DIR="$with_dbus_system_services"
+else
+DBUS_SYSTEM_SERVICES_DIR="$datadir/dbus-1/system-services"
+fi
+AC_SUBST(DBUS_SYSTEM_SERVICES_DIR)
+
 AC_OUTPUT(Makefile
   data/Makefile
   src/Makefile
diff --git a/data/Makefile.am b/data/Makefile.am
index b0b30b9..b8f1376 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -3,13 +3,25 @@ service_in_files = \
 servicedir = $(DBUS_SERVICES_DIR)
 service_DATA = $(service_in_files:.service.in=.service)
 
+system_service_in_files = \
+   system/org.libvirt.service.in
+system_servicedir = $(DBUS_SYSTEM_SERVICES_DIR)
+system_service_DATA = $(system_service_in_files:.service.in=.service)
+
 EXTRA_DIST = \
-   $(service_in_files)
+   $(service_in_files) \
+   $(system_service_in_files)
 
 CLEANFILES = \
-   $(service_DATA)
+   $(service_DATA) \
+   $(system_service_DATA)
 
 session/org.libvirt.service: session/org.libvirt.service.in
$(AM_V_GEN)$(MKDIR_P) session && \
sed -e 's|[@]bindir[@]|$(bindir)|g' < $< > $@-t && \
mv $@-t $@
+
+system/org.libvirt.service: system/org.libvirt.service.in
+   $(AM_V_GEN)$(MKDIR_P) system && \
+   sed -e 's|[@]bindir[@]|$(bindir)|g' < $< > $@-t && \
+   mv $@-t $@
diff --git a/data/system/org.libvirt.service.in 
b/data/system/org.libvirt.service.in
new file mode 100644
index 000..08d32a2
--- /dev/null
+++ b/data/system/org.libvirt.service.in
@@ -0,0 +1,4 @@
+[D-BUS Service]
+Name=org.libvirt
+Exec=@bindir@/libvirt-dbus --system
+User=root
diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in
index c92b320..0f76de2 100644
--- a/libvirt-dbus.spec.in
+++ b/libvirt-dbus.spec.in
@@ -42,5 +42,6 @@ rm -rf $RPM_BUILD_ROOT
 %doc README COPYING AUTHORS NEWS
 %{_bindir}/libvirt-dbus
 %{_datadir}/dbus-1/services/org.libvirt.service
+%{_datadir}/dbus-1/system-services/org.libvirt.service
 
 %changelog
-- 
2.13.4

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

[libvirt] [dbus PATCH 5/7] maint: move service file into data directory

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 .gitignore  |  3 ++-
 Makefile.am |  2 +-
 configure.ac|  2 ++
 data/Makefile.am| 15 +++
 data/session/org.libvirt.service.in |  3 +++
 src/Makefile.am | 17 -
 src/org.libvirt.service.in  |  3 ---
 7 files changed, 23 insertions(+), 22 deletions(-)
 create mode 100644 data/Makefile.am
 create mode 100644 data/session/org.libvirt.service.in
 delete mode 100644 src/org.libvirt.service.in

diff --git a/.gitignore b/.gitignore
index 72f3595..f223068 100644
--- a/.gitignore
+++ b/.gitignore
@@ -22,6 +22,7 @@ vgcore.*
 /libvirt-dbus.spec
 /stamp-h1
 
+/data/session/org.libvirt.service
+
 /src/.deps/
 /src/libvirt-dbus
-/src/org.libvirt.service
diff --git a/Makefile.am b/Makefile.am
index 8ba2137..065334f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,5 +1,5 @@
 
-SUBDIRS = src test
+SUBDIRS = data src test
 
 EXTRA_DIST = \
$(PACKAGE).spec \
diff --git a/configure.ac b/configure.ac
index 8de0d35..158c5e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -29,6 +29,7 @@ AC_SUBST([LIBVIRT_DBUS_VERSION_INFO])
 AC_SUBST([LIBVIRT_DBUS_VERSION_NUMBER])
 
 AC_PROG_CC
+AC_PROG_MKDIR_P
 AM_PROG_CC_C_O
 
 PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED)
@@ -45,6 +46,7 @@ fi
 AC_SUBST(DBUS_SERVICES_DIR)
 
 AC_OUTPUT(Makefile
+  data/Makefile
   src/Makefile
   test/Makefile
   libvirt-dbus.spec)
diff --git a/data/Makefile.am b/data/Makefile.am
new file mode 100644
index 000..b0b30b9
--- /dev/null
+++ b/data/Makefile.am
@@ -0,0 +1,15 @@
+service_in_files = \
+   session/org.libvirt.service.in
+servicedir = $(DBUS_SERVICES_DIR)
+service_DATA = $(service_in_files:.service.in=.service)
+
+EXTRA_DIST = \
+   $(service_in_files)
+
+CLEANFILES = \
+   $(service_DATA)
+
+session/org.libvirt.service: session/org.libvirt.service.in
+   $(AM_V_GEN)$(MKDIR_P) session && \
+   sed -e 's|[@]bindir[@]|$(bindir)|g' < $< > $@-t && \
+   mv $@-t $@
diff --git a/data/session/org.libvirt.service.in 
b/data/session/org.libvirt.service.in
new file mode 100644
index 000..a8cb6a9
--- /dev/null
+++ b/data/session/org.libvirt.service.in
@@ -0,0 +1,3 @@
+[D-BUS Service]
+Name=org.libvirt
+Exec=@bindir@/libvirt-dbus --session
diff --git a/src/Makefile.am b/src/Makefile.am
index 30e7a35..917c46d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -27,20 +27,3 @@ libvirt_dbus_LDFLAGS = \
 libvirt_dbus_LDADD = \
$(SYSTEMD_LIBS) \
$(LIBVIRT_LIBS)
-
-service_in_files = \
-   org.libvirt.service.in
-servicedir = $(DBUS_SERVICES_DIR)
-service_DATA = $(service_in_files:.service.in=.service)
-
-EXTRA_DIST += \
-   $(service_in_files)
-
-CLEANFILES += \
-   $(service_DATA)
-
-org.libvirt.service: org.libvirt.service.in
-   $(AM_V_GEN)sed \
-   -e 's|[@]bindir[@]|$(bindir)|g' \
-   < $< > $@-t && \
-   mv $@-t $@
diff --git a/src/org.libvirt.service.in b/src/org.libvirt.service.in
deleted file mode 100644
index a42539e..000
--- a/src/org.libvirt.service.in
+++ /dev/null
@@ -1,3 +0,0 @@
-[D-BUS Service]
-Name=org.libvirt
-Exec=@bindir@/libvirt-dbus
-- 
2.13.4

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

[libvirt] [dbus PATCH 4/7] build: move test related bits to test/Makefile.am

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 Makefile.am  | 19 +--
 configure.ac |  1 +
 test/Makefile.am | 16 
 3 files changed, 18 insertions(+), 18 deletions(-)
 create mode 100644 test/Makefile.am

diff --git a/Makefile.am b/Makefile.am
index 67e9d1e..8ba2137 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,5 +1,5 @@
 
-SUBDIRS = src
+SUBDIRS = src test
 
 EXTRA_DIST = \
$(PACKAGE).spec \
@@ -24,20 +24,3 @@ gen-AUTHORS:
< $(srcdir)/AUTHORS.in > $(distdir)/AUTHORS-tmp &&\
  mv -f $(distdir)/AUTHORS-tmp $(distdir)/AUTHORS ;   \
fi
-
-test_helpers = \
-   test/libvirttest.py
-
-test_programs = \
-   test/test_manager.py \
-   test/test_domain.py
-
-TESTS = $(test_programs)
-
-EXTRA_DIST += \
-   $(test_helpers) \
-   $(test_programs) \
-   test/travis-run
-
-TESTS_ENVIRONMENT = \
-   abs_top_builddir=$(abs_top_builddir)
diff --git a/configure.ac b/configure.ac
index 6a123fb..8de0d35 100644
--- a/configure.ac
+++ b/configure.ac
@@ -46,4 +46,5 @@ AC_SUBST(DBUS_SERVICES_DIR)
 
 AC_OUTPUT(Makefile
   src/Makefile
+  test/Makefile
   libvirt-dbus.spec)
diff --git a/test/Makefile.am b/test/Makefile.am
new file mode 100644
index 000..a7f22d0
--- /dev/null
+++ b/test/Makefile.am
@@ -0,0 +1,16 @@
+test_helpers = \
+   libvirttest.py
+
+test_programs = \
+   test_manager.py \
+   test_domain.py
+
+TESTS = $(test_programs)
+
+EXTRA_DIST = \
+   $(test_helpers) \
+   $(test_programs) \
+   travis-run
+
+TESTS_ENVIRONMENT = \
+   abs_top_builddir=$(abs_top_builddir)
-- 
2.13.4

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

[libvirt] [dbus PATCH 3/7] build: fix distcheck

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 Makefile.am | 15 ++-
 test/libvirttest.py |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index abb154c..67e9d1e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -25,6 +25,19 @@ gen-AUTHORS:
  mv -f $(distdir)/AUTHORS-tmp $(distdir)/AUTHORS ;   \
fi
 
-TESTS = \
+test_helpers = \
+   test/libvirttest.py
+
+test_programs = \
test/test_manager.py \
test/test_domain.py
+
+TESTS = $(test_programs)
+
+EXTRA_DIST += \
+   $(test_helpers) \
+   $(test_programs) \
+   test/travis-run
+
+TESTS_ENVIRONMENT = \
+   abs_top_builddir=$(abs_top_builddir)
diff --git a/test/libvirttest.py b/test/libvirttest.py
index 706b352..f5c2020 100644
--- a/test/libvirttest.py
+++ b/test/libvirttest.py
@@ -8,7 +8,7 @@ import subprocess
 import time
 import unittest
 
-root = os.path.dirname(os.path.dirname(__file__))
+root = os.environ.get('abs_top_builddir', 
os.path.dirname(os.path.dirname(__file__)))
 exe = os.path.join(root, 'src', 'libvirt-dbus')
 
 DBusGMainLoop(set_as_default=True)
-- 
2.13.4

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

[libvirt] [dbus PATCH 0/7] additional cleanups and improvements

[Pavel Hrdina]

Pavel Hrdina (7):
  maint: cleanup gitignore
  maint: ignore dist tarball
  build: fix distcheck
  build: move test related bits to test/Makefile.am
  maint: move service file into data directory
  data: add system dbus service file
  data: add system dbus service policy configuration

 .gitignore  | 21 +++--
 Makefile.am |  6 +-
 configure.ac| 23 +++
 data/Makefile.am| 33 +
 data/session/org.libvirt.service.in |  3 +++
 data/system/org.libvirt.conf| 12 
 data/system/org.libvirt.service.in  |  4 
 libvirt-dbus.spec.in|  2 ++
 src/Makefile.am | 17 -
 src/org.libvirt.service.in  |  3 ---
 test/Makefile.am| 16 
 test/libvirttest.py |  2 +-
 12 files changed, 106 insertions(+), 36 deletions(-)
 create mode 100644 data/Makefile.am
 create mode 100644 data/session/org.libvirt.service.in
 create mode 100644 data/system/org.libvirt.conf
 create mode 100644 data/system/org.libvirt.service.in
 delete mode 100644 src/org.libvirt.service.in
 create mode 100644 test/Makefile.am

-- 
2.13.4

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

[libvirt] [dbus PATCH 1/7] maint: cleanup gitignore

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 .gitignore | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0844b58..e4bd377 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,15 +1,15 @@
+*.log
 *.o
-*~
-vgcore.*
 *.pyc
-__pycache__
-*.log
 *.trs
+*Makefile
+*Makefile.in
+*~
+__pycache__
+vgcore.*
 
 /AUTHORS
 /INSTALL
-/Makefile
-/Makefile.in
 /aclocal.m4
 /autom4te.cache/
 /build-aux/
@@ -19,10 +19,8 @@ __pycache__
 /config.status
 /configure
 /libvirt-dbus.spec
-/src/.deps/
-/src/Makefile
-/src/Makefile.in
 /stamp-h1
 
+/src/.deps/
 /src/libvirt-dbus
 /src/org.libvirt.service
-- 
2.13.4

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

[libvirt] [dbus PATCH 2/7] maint: ignore dist tarball

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index e4bd377..72f3595 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,6 +18,7 @@ vgcore.*
 /config.log
 /config.status
 /configure
+/libvirt-dbus-*.tar.gz
 /libvirt-dbus.spec
 /stamp-h1
 
-- 
2.13.4

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

[libvirt] [PATCH] qemuBuildMemoryBackendStr: Handle one more corner case

[Michal Privoznik]

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

This code is so complicated because we allow enabling the same
bits at many places. Just like in this case: huge pages can be
enabled by global  element under  or
on per  basis. To complicate things a bit more, users
are allowed to not specify any specific page size in which case
the default page size is used. And this is what is causing this
bug. If no page size is specified, @pagesize is keeping value of
zero throughout whole function. Therefore we need yet another
boolean to hold [use, don't use] information as we can't sue
@pagesize for that.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c| 22 ++--
 .../qemuxml2argv-hugepages-pages7.args | 35 
 .../qemuxml2argv-hugepages-pages7.xml  | 65 ++
 tests/qemuxml2argvtest.c   |  2 +
 .../qemuxml2xmlout-hugepages-pages7.xml|  1 +
 tests/qemuxml2xmltest.c|  1 +
 6 files changed, 120 insertions(+), 6 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml
 create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 604650596..68fc13706 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3250,8 +3250,6 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
   virBitmapPtr autoNodeset,
   bool force)
 {
-virDomainHugePagePtr master_hugepage = NULL;
-virDomainHugePagePtr hugepage = NULL;
 virDomainNumatuneMemMode mode;
 const long system_page_size = virGetSystemPageSizeKB();
 virDomainMemoryAccess memAccess = mem->access;
@@ -3264,6 +3262,13 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
 bool nodeSpecified = virDomainNumatuneNodeSpecified(def->numa, 
mem->targetNode);
 unsigned long long pagesize = mem->pagesize;
 bool needHugepage = !!pagesize;
+bool useHugepage = !!pagesize;
+
+/* The difference between @needHugepage and @useHugepage is that the latter
+ * is true whenever huge page is defined for the current memory cell.
+ * Either directly, or transitively via global domain huge pages. The
+ * former is true whenever "memory-backend-file" must be used to satisfy
+ * @useHugepage. */
 
 *backendProps = NULL;
 *backendType = NULL;
@@ -3290,6 +3295,8 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
 mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
 
 if (pagesize == 0) {
+virDomainHugePagePtr master_hugepage = NULL;
+virDomainHugePagePtr hugepage = NULL;
 bool thisHugepage = false;
 
 /* Find the huge page size we want to use */
@@ -3327,8 +3334,10 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
 hugepage = master_hugepage;
 }
 
-if (hugepage)
+if (hugepage) {
 pagesize = hugepage->size;
+useHugepage = true;
+}
 }
 
 if (pagesize == system_page_size) {
@@ -3336,17 +3345,18 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
  * of regular system page size, it's as if they
  * hasn't specified any huge pages at all. */
 pagesize = 0;
-hugepage = NULL;
+needHugepage = false;
+useHugepage = false;
 }
 
 if (!(props = virJSONValueNewObject()))
 return -1;
 
-if (pagesize || mem->nvdimmPath || memAccess ||
+if (useHugepage || mem->nvdimmPath || memAccess ||
 def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
 *backendType = "memory-backend-file";
 
-if (pagesize) {
+if (useHugepage) {
 if (qemuGetDomainHupageMemPath(def, cfg, pagesize, ) < 0)
 goto cleanup;
 prealloc = true;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args
new file mode 100644
index 0..e4229dce6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name fedora \
+-S \
+-M pc-i440fx-2.3 \
+-m size=1048576k,slots=16,maxmem=1099511627776k \
+-smp 2,sockets=2,cores=1,threads=1 \
+-mem-prealloc \
+-mem-path /dev/hugepages2M/libvirt/qemu/-1-fedora \
+-numa node,nodeid=0,cpus=0-1,mem=1024 \
+-object memory-backend-file,id=memdimm0,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu/-1-fedora,size=1073741824,\
+host-nodes=1-3,policy=bind \
+-device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \
+-object memory-backend-file,id=memdimm1,prealloc=yes,\

Re: [libvirt] [PATCH] docs: Add "PCI topology and hotplug" guidelines

[Andrea Bolognani]

On Tue, 2017-08-08 at 10:06 -0400, Laine Stump wrote:
> > +  The reason for this apparent limitation is the fact that each
> > +  hotplugged PCI device might require additional PCI controllers to
> > +  be added to the guest, and libvirt has no way of knowing in advance
> > +  how many devices will be hotplugged during the guest's lifetime,
> > +  thus making it impossible to automatically provide the right amount
> > +  of PCI controllers: any arbitrary number would end up being too big
> > +  for some users, and too small for others.
> 
> Of course we all know this, but you haven't said here that PCI
> controllers in general cannot themselves be hotplugged (although the new
> pcie-pci-bridge *will* be hotpluggable, as long as the OS supports that).

That's a good point, I'll mention it.

> > +q35 machine type
> > +
> > +
> > +  This is a PCI Express native machine type. The default PCI topology
> > +  looks like
> > +
> > +
> > +
> > +controller type='pci' index='0' model='pcie-root'/
> > +controller type='pci' index='1' model='pcie-root-port'
> > +  model name='pcie-root-port'/
> > +  target chassis='1' port='0x10'/
> > +  address type='pci' domain='0x' bus='0x00' slot='0x01' 
> > function='0x0'/
> > +/controller
> > +
> > +
> > +  and supports hotplugging a single PCI Express device, either
> > +  emulated or assigned from the host.
> > +
> 
> Didn't we include some trick in there (requested for libguestfs
> appliances) that allows creating a config that has no pcie-root-ports?

Yeah, we had something like that but I can't recall the
specifics at the moment. I'll look it up and add a note
about it.

> > +  pcie-root-port controller. If you plan to hotplug
> > +  more than a single PCI Express device, you should add a suitable
> > +  number of pcie-root-port controllers when defining
> > +  the guest: for example, add
> > +
> > +
> > +
> > +controller type='pci' model='pcie-root-port'/
> > +controller type='pci' model='pcie-root-port'/
> > +controller type='pci' model='pcie-root-port'/
> > +
> > +
> > +  if you expect to hotplug up to three PCI Express devices,
> > +  either emulated or assigned from the host. That's all the
> > +  information you need to provide: libvirt will fill in the
> > +  remaining details automatically.
> > +
> 
> Maybe a note here pointing out that if you add root-ports and new
> endpoint devices at the same time, the endpoint devices will
> automatically be attached to the manually added root-ports, so if you're
> trying to end up with spares, you'll need to manually add enough for the
> endpoints, plus the number of spares you want (you won't need to address
> any of the controllers or endpoints though).

Sure.

> > +
> > +  If you expect to hotplug legacy PCI devices, then you will need
> > +  specialized controllers, since all those mentioned above are
> > +  intended for PCI Express devices only: add
> > +
> > +
> > +
> > +controller type='pci' model='dmi-to-pci-bridge'/
> > +controller type='pci' model='pci-bridge'/
> > +
> > +
> > +  and you'll be able to hotplug up to 31 legacy PCI devices,
> > +  either emulated or assigned from the host.
> > +
> 
> Maybe mention that it's slot 1 - 31 because slot 0 is reserved.

Not sure that's necessarily in scope for the document, since
in general you'll let libvirt pick the slot for you.

That said, adding it would hardly make a difference when it
comes to document size, so why not I guess :)

> > +
> > +  The default PCI topology for the pseries machine
> > +  type looks like
> > +
> > +
> > +
> > +controller type='pci' index='0' model='pcie-root'
> 
> You mean pci-root, right? (I always get these mixed up for PPC, since I
> don't actually use it).

Of course I did, and in fact I got it right right afterwards.
Nice catch :)

> There's of course always more that can be written, but this is a good
> and useful start, so ACK (with the few typos fixed).

Thanks, I've pushed it. I'll address the improvements you
suggested 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] [PATCH] docs: Add "PCI topology and hotplug" guidelines

[Laine Stump]

On 07/25/2017 06:42 AM, Andrea Bolognani wrote:
> For all machine types except i440fx, making a guest hotplug
> capable requires some sort of planning. Add some information
> to help users make educated choices when defining the PCI
> topology of guests.
>
> Signed-off-by: Andrea Bolognani 
> ---
>  docs/formatdomain.html.in |   4 +-
>  docs/pci-hotplug.html.in  | 164 
> ++
>  2 files changed, 167 insertions(+), 1 deletion(-)
>  create mode 100644 docs/pci-hotplug.html.in
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index bceddd2..7c4450c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3505,7 +3505,9 @@
>appear more than once, with a group of virtual devices tied to a
>virtual controller.  Normally, libvirt can automatically infer such
>controllers without requiring explicit XML markup, but sometimes
> -  it is necessary to provide an explicit controller element.
> +  it is necessary to provide an explicit controller element, notably
> +  when planning the PCI topology
> +  for guests where device hotplug is expected.
>  
>  
>  
> diff --git a/docs/pci-hotplug.html.in b/docs/pci-hotplug.html.in
> new file mode 100644
> index 000..f3d1610
> --- /dev/null
> +++ b/docs/pci-hotplug.html.in
> @@ -0,0 +1,164 @@
> +
> + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd;>
> +http://www.w3.org/1999/xhtml;>
> +  
> +PCI topology and hotplug
> +
> +
> +
> +
> +  Perhaps surprisingly, most libvirt guests support only limited PCI
> +  device hotplug out of the box, or even none at all.
> +
> +
> +  The reason for this apparent limitation is the fact that each
> +  hotplugged PCI device might require additional PCI controllers to
> +  be added to the guest, and libvirt has no way of knowing in advance
> +  how many devices will be hotplugged during the guest's lifetime,
> +  thus making it impossible to automatically provide the right amount
> +  of PCI controllers: any arbitrary number would end up being too big
> +  for some users, and too small for others.

Of course we all know this, but you haven't said here that PCI
controllers in general cannot themselves be hotplugged (although the new
pcie-pci-bridge *will* be hotpluggable, as long as the OS supports that).

> +
> +
> +  Ultimately, the user is the only one who knows how much the guest
> +  will need to grow dynamically, so the responsability of planning

s/responsability/responsibility/

> +  a suitabile PCI topology in advance falls on them.

s/suitabile/suitable/

> +
> +
> +  This document aims at providing all the information needed to
> +  successfully plan the PCI topology of a guest. Note that the
> +  details can vary a lot between architectures and even machine
> +  types, hence the way it's organized.
> +
> +
> +x86_64 architecture
> +
> +q35 machine type
> +
> +
> +  This is a PCI Express native machine type. The default PCI topology
> +  looks like
> +
> +
> +
> +controller type='pci' index='0' model='pcie-root'/
> +controller type='pci' index='1' model='pcie-root-port'
> +  model name='pcie-root-port'/
> +  target chassis='1' port='0x10'/
> +  address type='pci' domain='0x' bus='0x00' slot='0x01' 
> function='0x0'/
> +/controller
> +
> +
> +  and supports hotplugging a single PCI Express device, either
> +  emulated or assigned from the host.
> +

Didn't we include some trick in there (requested for libguestfs
appliances) that allows creating a config that has no pcie-root-ports?


> +
> +  Slots on the pcie-root controller do not support
> +  hotplug, so the device will be hotplugged into the



> +  pcie-root-port controller. If you plan to hotplug
> +  more than a single PCI Express device, you should add a suitable
> +  number of pcie-root-port controllers when defining
> +  the guest: for example, add
> +
> +
> +
> +controller type='pci' model='pcie-root-port'/
> +controller type='pci' model='pcie-root-port'/
> +controller type='pci' model='pcie-root-port'/
> +
> +
> +  if you expect to hotplug up to three PCI Express devices,
> +  either emulated or assigned from the host. That's all the
> +  information you need to provide: libvirt will fill in the
> +  remaining details automatically.
> +

Maybe a note here pointing out that if you add root-ports and new
endpoint devices at the same time, the endpoint devices will
automatically be attached to the manually added root-ports, so if you're
trying to end up with spares, you'll need to manually add enough for the
endpoints, plus the number of spares you want (you won't need to address
any of the controllers or endpoints though).


> +
> +  If you expect to hotplug legacy PCI devices, then you will need
> +  

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Gao, Ping A]

On 2017/8/8 14:42, Kirti Wankhede wrote:
>
> On 8/7/2017 1:11 PM, Gao, Ping A wrote:
>> On 2017/8/4 5:11, Alex Williamson wrote:
>>> On Thu, 3 Aug 2017 20:26:14 +0800
>>> "Gao, Ping A"  wrote:
>>>
 On 2017/8/3 0:58, Alex Williamson wrote:
> On Wed, 2 Aug 2017 21:16:28 +0530
> Kirti Wankhede  wrote:
>  
>> On 8/2/2017 6:29 PM, Gao, Ping A wrote:  
>>> On 2017/8/2 18:19, Kirti Wankhede wrote:
 On 8/2/2017 3:56 AM, Alex Williamson wrote:
> On Tue, 1 Aug 2017 13:54:27 +0800
> "Gao, Ping A"  wrote:
>
>> On 2017/7/28 0:00, Gao, Ping A wrote:
>>> On 2017/7/27 0:43, Alex Williamson wrote:  
 [cc +libvir-list]

 On Wed, 26 Jul 2017 21:16:59 +0800
 "Gao, Ping A"  wrote:
  
> The vfio-mdev provide the capability to let different guest share 
> the
> same physical device through mediate sharing, as result it bring a
> requirement about how to control the device sharing, we need a QoS
> related interface for mdev to management virtual device resource.
>
> E.g. In practical use, vGPUs assigned to different quests almost 
> has
> different performance requirements, some guests may need higher 
> priority
> for real time usage, some other may need more portion of the GPU
> resource to get higher 3D performance, corresponding we can 
> define some
> interfaces like weight/cap for overall budget control, priority 
> for
> single submission control.
>
> So I suggest to add some common attributes which are vendor 
> agnostic in
> mdev core sysfs for QoS purpose.  
 I think what you're asking for is just some standardization of a 
 QoS
 attribute_group which a vendor can optionally include within the
 existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
 transparently enable this, but it really only provides the 
 standard,
 all of the support code is left for the vendor.  I'm fine with 
 that,
 but of course the trouble with and sort of standardization is 
 arriving
 at an agreed upon standard.  Are there QoS knobs that are generic
 across any mdev device type?  Are there others that are more 
 specific
 to vGPU?  Are there existing examples of this that we can steal 
 their
 specification?  
>>> Yes, you are right, standardization QoS knobs are exactly what I 
>>> wanted.
>>> Only when it become a part of the mdev framework and libvirt, then 
>>> QoS
>>> such critical feature can be leveraged by cloud usage. HW vendor 
>>> only
>>> need to focus on the implementation of the corresponding QoS 
>>> algorithm
>>> in their back-end driver.
>>>
>>> Vfio-mdev framework provide the capability to share the device that 
>>> lack
>>> of HW virtualization support to guests, no matter the device type,
>>> mediated sharing actually is a time sharing multiplex method, from 
>>> this
>>> point of view, QoS can be take as a generic way about how to 
>>> control the
>>> time assignment for virtual mdev device that occupy HW. As result 
>>> we can
>>> define QoS knob generic across any device type by this way. Even if 
>>> HW
>>> has build in with some kind of QoS support, I think it's not a 
>>> problem
>>> for back-end driver to convert mdev standard QoS definition to their
>>> specification to reach the same performance expectation. Seems 
>>> there are
>>> no examples for us to follow, we need define it from scratch.
>>>
>>> I proposal universal QoS control interfaces like below:
>>>
>>> Cap: The cap limits the maximum percentage of time a mdev device 
>>> can own
>>> physical device. e.g. cap=60, means mdev device cannot take over 
>>> 60% of
>>> total physical resource.
>>>
>>> Weight: The weight define proportional control of the mdev device
>>> resource between guests, it’s orthogonal with Cap, to target load
>>> balancing. E.g. if guest 1 should take double mdev device resource
>>> compare with guest 2, need set weight ratio to 2:1.
>>>
>>> Priority: The guest who has higher priority will get execution 
>>> first,
>>> target to some real time usage and speeding interactive response.
>>>
>>> 

[libvirt] [PATCH v4 5/7] virsh: Implement managedsave-define command

[Kothapally Madhu Pavan]

Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan 
---
 tools/virsh-domain.c | 79 
 tools/virsh.pod  | 14 ++
 2 files changed, 93 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 512804c..8baea2a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4705,6 +4705,79 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "managedsave-define" command
+ */
+static const vshCmdInfo info_managed_save_define[] = {
+{.name = "help",
+ .data = N_("redefine the XML for a domain's managed save state file")
+},
+{.name = "desc",
+ .data = N_("Replace the domain XML associated with a managed save state 
file")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_managed_save_define[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL,
+{.name = "xml",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("filename containing updated XML for the target")
+},
+{.name = "running",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be running on start")
+},
+{.name = "paused",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be paused on start")
+},
+{.name = NULL}
+};
+
+static bool
+cmdManagedSaveDefine(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+virDomainPtr dom = NULL;
+const char *xmlfile = NULL;
+char *xml = NULL;
+unsigned int flags = 0;
+
+if (vshCommandOptBool(cmd, "running"))
+flags |= VIR_DOMAIN_SAVE_RUNNING;
+if (vshCommandOptBool(cmd, "paused"))
+flags |= VIR_DOMAIN_SAVE_PAUSED;
+
+VSH_EXCLUSIVE_OPTIONS("running", "paused");
+
+if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
+return false;
+
+if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, ) < 0)
+return false;
+
+dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+if (virDomainManagedSaveDefineXML(dom, xml, flags) < 0) {
+vshError(ctl, _("Failed to update %s XML configuration"),
+virDomainGetName(dom));
+goto cleanup;
+}
+
+vshPrintExtra(ctl, _("Managed save state file of domain %s updated.\n"),
+ virDomainGetName(dom));
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+VIR_FREE(xml);
+return ret;
+}
+
+/*
  * "schedinfo" command
  */
 static const vshCmdInfo info_schedinfo[] = {
@@ -13845,6 +13918,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_managedsaveremove,
  .flags = 0
 },
+{.name = "managedsave-define",
+ .handler = cmdManagedSaveDefine,
+ .opts = opts_managed_save_define,
+ .info = info_managed_save_define,
+ .flags = 0
+},
 {.name = "memtune",
  .handler = cmdMemtune,
  .opts = opts_memtune,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 43d6f0c..11e2321 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1630,6 +1630,20 @@ has any managed save image.
 Remove the B state file for a domain, if it exists.  This
 ensures the domain will do a full boot the next time it is started.
 
+=item B I I [{I<--running> | I<--paused>}]
+
+Update the domain XML that will be used when I is later
+started. The I argument must be a file name containing
+the alternative XML, with changes only in the host-specific portions of
+the domain XML. For example, it can be used to account for file naming
+differences resulting from creating disk snapshots of underlying storage
+after the guest was saved.
+
+The managed save image records whether the domain should be started to a
+running or paused state.  Normally, this command does not alter the
+recorded state; passing either the I<--running> or I<--paused> flag
+will allow overriding which state the B should use.
+
 =item B [I]
 
 Provide the maximum number of virtual CPUs supported for a guest VM on
-- 
1.8.3.1

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

[libvirt] [PATCH v4 3/7] qemu: Implement qemuDomainManagedSaveGetXMLDesc

[Kothapally Madhu Pavan]

This commit adds qemu driver implementation to get xml description
for managed save state domain.

Signed-off-by: Kothapally Madhu Pavan 
---
 src/qemu/qemu_driver.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f65f4..ec73dc1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6797,6 +6797,51 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
 return ret;
 }
 
+static char *
+qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm;
+char *path = NULL;
+char *ret = NULL;
+virDomainDefPtr def = NULL;
+int fd = -1;
+virQEMUSaveDataPtr data = NULL;
+
+/* We only take subset of virDomainDefFormat flags.  */
+virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return ret;
+
+path = qemuDomainManagedSavePath(driver, vm);
+
+if (!path)
+goto cleanup;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain does not have managed save image"));
+goto cleanup;
+}
+
+fd = qemuDomainSaveImageOpen(driver, path, , ,
+ false, NULL, false, false);
+if (fd < 0)
+goto cleanup;
+if (virDomainManagedSaveGetXMLDescEnsureACL(dom->conn, def, flags) < 0)
+goto cleanup;
+ret = qemuDomainDefFormatXML(driver, def, flags);
+
+ cleanup:
+virQEMUSaveDataFree(data);
+virDomainDefFree(def);
+VIR_FORCE_CLOSE(fd);
+virDomainObjEndAPI();
+VIR_FREE(path);
+return ret;
+}
+
 /* Return 0 on success, 1 if incomplete saved image was silently unlinked,
  * and -1 on failure with error raised.  */
 static int
@@ -20839,6 +20884,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainManagedSave = qemuDomainManagedSave, /* 0.8.0 */
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
+.domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.7.0 */
 .domainSnapshotCreateXML = qemuDomainSnapshotCreateXML, /* 0.8.0 */
 .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */
 .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] Update to latest keycodemapdb content

[Andrea Bolognani]

On Mon, 2017-08-07 at 14:38 +0100, Daniel P. Berrange wrote:
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/Makefile.am   | 2 +-
>  src/keycodemapdb  | 2 +-
>  src/util/virkeycode.c | 5 ++---
>  3 files changed, 4 insertions(+), 5 deletions(-)

The changes look sane, but the test suite is not happy:

  $ VIR_TEST_DEBUG=1 ./tests/virkeycodetest
  TEST: virkeycodetest
   1) Keycode mapping ... Translating 259 from ATSET2
  to ATSET3, got -1 want 55
  FAILED

Seems unrelated to your changes though, perhaps a
regression in keycodemapdb?

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH v4 1/7] lib: Add API to dump xml configuration of managed save state domain

[Kothapally Madhu Pavan]

Similar to domainSaveImageGetXMLDesc this commit adds 
domainManagedSaveGetXMLDesc
API which allows to get the xml of managed save state domain.

Signed-off-by: Kothapally Madhu Pavan 
---
 include/libvirt/libvirt-domain.h |  2 ++
 src/driver-hypervisor.h  |  5 
 src/libvirt-domain.c | 49 
 src/libvirt_public.syms  |  5 
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 19 ++--
 src/remote_protocol-structs  |  8 +++
 7 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index a3bb9cb..8ede912 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1209,6 +1209,8 @@ int
virDomainHasManagedSaveImage(virDomainPtr dom,
 unsigned int flags);
 intvirDomainManagedSaveRemove(virDomainPtr dom,
   unsigned int flags);
+char * virDomainManagedSaveGetXMLDesc(virDomainPtr domain,
+  unsigned int flags);
 
 /*
  * Domain core dump
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 3053d7a..598fc06 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -745,6 +745,10 @@ typedef int
 (*virDrvDomainManagedSaveRemove)(virDomainPtr domain,
  unsigned int flags);
 
+typedef char *
+(*virDrvDomainManagedSaveGetXMLDesc)(virDomainPtr domain,
+ unsigned int flags);
+
 typedef virDomainSnapshotPtr
 (*virDrvDomainSnapshotCreateXML)(virDomainPtr domain,
  const char *xmlDesc,
@@ -1422,6 +1426,7 @@ struct _virHypervisorDriver {
 virDrvDomainManagedSave domainManagedSave;
 virDrvDomainHasManagedSaveImage domainHasManagedSaveImage;
 virDrvDomainManagedSaveRemove domainManagedSaveRemove;
+virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc;
 virDrvDomainSnapshotCreateXML domainSnapshotCreateXML;
 virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc;
 virDrvDomainSnapshotNum domainSnapshotNum;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 87fca29..cb260e2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9298,6 +9298,55 @@ virDomainManagedSaveRemove(virDomainPtr dom, unsigned 
int flags)
 }
 
 
+/**
+ * virDomainManagedSaveGetXMLDesc:
+ * @domain: a domain object
+ * @flags: bitwise-OR of subset of virDomainXMLFlags
+ *
+ * This method will extract the XML description of the managed save
+ * state file of a domain.
+ *
+ * No security-sensitive data will be included unless @flags contains
+ * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
+ * connections.  For this API, @flags should not contain either
+ * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ *
+ * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
+ * error.  The caller must free() the returned value.
+ */
+char *
+virDomainManagedSaveGetXMLDesc(virDomainPtr domain, unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "flags=%x", flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, NULL);
+conn = domain->conn;
+
+if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) {
+virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+   _("virDomainManagedSaveGetXMLDesc with secure flag"));
+goto error;
+}
+
+if (conn->driver->domainManagedSaveGetXMLDesc) {
+char *ret;
+ret = conn->driver->domainManagedSaveGetXMLDesc(domain, flags);
+if (!ret)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return NULL;
+}
+
 
 /**
  * virDomainOpenConsole:
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index fac77fb..b38d0bb 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -768,4 +768,9 @@ LIBVIRT_3.4.0 {
 virStreamSparseSendAll;
 } LIBVIRT_3.1.0;
 
+LIBVIRT_3.7.0 {
+global:
+virDomainManagedSaveGetXMLDesc;
+} LIBVIRT_3.4.0;
+
 #  define new API here using predicted next version number 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a57d25f..7a74b87 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8410,6 +8410,7 @@ static virHypervisorDriver hypervisor_driver = {
 .domainManagedSave = remoteDomainManagedSave, /* 0.8.0 */
 .domainHasManagedSaveImage = remoteDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = remoteDomainManagedSaveRemove, /* 0.8.0 */
+.domainManagedSaveGetXMLDesc = remoteDomainManagedSaveGetXMLDesc, /* 3.7.0 
*/
 

[libvirt] [PATCH v4 6/7] virsh: Implement managedsave-dumpxml command

[Kothapally Madhu Pavan]

Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan 
---
 tools/virsh-domain.c | 56 
 tools/virsh.pod  |  6 ++
 2 files changed, 62 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8baea2a..ce15090 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4705,6 +4705,56 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "managedsave-dumpxml" command
+ */
+static const vshCmdInfo info_managed_save_dumpxml[] = {
+   {.name = "help",
+.data = N_("Domain information of managed save state file in XML")
+   },
+   {.name = "desc",
+.data = N_("Dump XML of domain information for a managed save state file 
to stdout.")
+   },
+   {.name = NULL}
+};
+
+static const vshCmdOptDef opts_managed_save_dumpxml[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL,
+{.name = "security-info",
+ .type = VSH_OT_BOOL,
+ .help = N_("include security sensitive information in XML dump")
+},
+{.name = NULL}
+};
+
+static bool
+cmdManagedSaveDumpxml(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+virDomainPtr dom = NULL;
+unsigned int flags = 0;
+char *xml = NULL;
+
+if (vshCommandOptBool(cmd, "security-info"))
+flags |= VIR_DOMAIN_XML_SECURE;
+
+dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+xml = virDomainManagedSaveGetXMLDesc(dom, flags);
+if (!xml)
+goto cleanup;
+
+vshPrint(ctl, "%s", xml);
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+VIR_FREE(xml);
+return ret;
+}
+
+/*
  * "managedsave-define" command
  */
 static const vshCmdInfo info_managed_save_define[] = {
@@ -13918,6 +13968,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_managedsaveremove,
  .flags = 0
 },
+{.name = "managedsave-dumpxml",
+ .handler = cmdManagedSaveDumpxml,
+ .opts = opts_managed_save_dumpxml,
+ .info = info_managed_save_dumpxml,
+ .flags = 0
+},
 {.name = "managedsave-define",
  .handler = cmdManagedSaveDefine,
  .opts = opts_managed_save_define,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 11e2321..1bda44f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1644,6 +1644,12 @@ running or paused state.  Normally, this command does 
not alter the
 recorded state; passing either the I<--running> or I<--paused> flag
 will allow overriding which state the B should use.
 
+=item B I [I<--security-info>]
+
+Extract the domain XML that was in effect at the time the saved state
+file I was created with the B command.  Using
+I<--security-info> will also include security sensitive information.
+
 =item B [I]
 
 Provide the maximum number of virtual CPUs supported for a guest VM on
-- 
1.8.3.1

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

[libvirt] [PATCH v4 0/7] Add new APIs to edit xml configuration of managed save state of a domain

[Kothapally Madhu Pavan]

managedsave command offloads the user from managing the save state file.
It does not need the user to specify saved state file location, all it takes
is domain name to identify. This makes it much more comfortable to use in
emergency where immediate shutdowm is needed. But it doesn't provide a way
to edit XML description of the save state file without user going through an
extra effort to search manually where the file actually exists.

The series aims to overcome the above constraints by adding new APIs and
commands to seemlessly edit the managed save state XML description using
just the domain name. The Patches mainly make use of the save-image-edit
code flow only to simplify the above use case.

This patch set provides capability to Dump and Edit the XML configuration
associated with a saved state file of a domain which was created by the
managedsave command.

The new command carry the similar options as the save-image- commands
to change the running state as to paused state or running on start.

This is equivalent to:

 virsh managedsave-dumpxml domain-name > state-file.xml
 vi state-file.xml (or make changes with your other text editor)
 virsh managedsave-define domain-name state-file-xml

or you can simply use:

 virsh managedsave-edit domain-name

It's always better when we get more.

Changes since v3:
- refracted version references from 3.6.0 to 3.7.0
- fixed typo in error message.

Changes since v2:
- refracted version references from 3.5.0 to 3.6.0

Changes since v1:
- qemu implementation called directly rather than going through
  driver pointer in qemuDomainManagedSaveDefineXML.
- check whether the managed save state file exists and report a
  error if it doesn't.

Kothapally Madhu Pavan (7):
  lib: Add API to dump xml configuration of managed save state domain
  lib: Add API to edit domain's managed save state xml configuration
  qemu: Implement qemuDomainManagedSaveGetXMLDesc
  qemu: Implement qemuDomainManagedSaveDefineXML
  virsh: Implement managedsave-define command
  virsh: Implement managedsave-dumpxml command
  virsh: Implement managedsave-edit command

 include/libvirt/libvirt-domain.h |   6 ++
 src/driver-hypervisor.h  |  11 +++
 src/libvirt-domain.c | 107 
 src/libvirt_public.syms  |   6 ++
 src/qemu/qemu_driver.c   |  87 
 src/remote/remote_driver.c   |   2 +
 src/remote/remote_protocol.x |  31 +-
 src/remote_protocol-structs  |  14 +++
 tools/virsh-domain.c | 207 +++
 tools/virsh.pod  |  41 
 10 files changed, 511 insertions(+), 1 deletion(-)

-- 
1.8.3.1

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

[libvirt] [PATCH v4 4/7] qemu: Implement qemuDomainManagedSaveDefineXML

[Kothapally Madhu Pavan]

This commit adds qemu driver implementation to edit xml
configuration of managed save state file of a domain.

Signed-off-by: Kothapally Madhu Pavan 
---
 src/qemu/qemu_driver.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec73dc1..b6db435 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6842,6 +6842,46 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, 
unsigned int flags)
 return ret;
 }
 
+static int
+qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml,
+   unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virConnectPtr conn = dom->conn;
+virDomainObjPtr vm;
+char *path = NULL;
+int ret;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+path = qemuDomainManagedSavePath(driver, vm);
+virDomainObjEndAPI();
+
+if (!path)
+goto error;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain does not have managed save image"));
+goto error;
+}
+
+ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags);
+
+VIR_FREE(path);
+
+if (ret < 0)
+goto error;
+
+return ret;
+
+ error:
+VIR_FREE(path);
+virDispatchError(conn);
+return -1;
+}
+
 /* Return 0 on success, 1 if incomplete saved image was silently unlinked,
  * and -1 on failure with error raised.  */
 static int
@@ -20885,6 +20925,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
 .domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.7.0 */
+.domainManagedSaveDefineXML = qemuDomainManagedSaveDefineXML, /* 3.7.0 */
 .domainSnapshotCreateXML = qemuDomainSnapshotCreateXML, /* 0.8.0 */
 .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */
 .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */
-- 
1.8.3.1

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

[libvirt] [PATCH] docs: force content in

[Daniel P. Berrange]

If there's no content in , the XSTL generator
will turn it into  which is not permitted in XHTML.
Adding a single whitespace is enough to guarantee an explicit
closing tag. Without this, the scripts never get loaded by
the browser.

Signed-off-by: Daniel P. Berrange 
---

Pushed as trivial fix

 docs/index.html.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/index.html.in b/docs/index.html.in
index 8333cf6bb..c0c55cb14 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -2,9 +2,9 @@
 
 http://www.w3.org/1999/xhtml;>
   
-
-
-
+ 
+ 
+ 
 
 

Re: [libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

[Michal Privoznik]

On 08/08/2017 10:49 AM, Pavel Hrdina wrote:
> On Mon, Aug 07, 2017 at 05:50:51PM +0200, Michal Privoznik wrote:
>> On 08/07/2017 05:30 PM, Pavel Hrdina wrote:
>>> On Mon, Aug 07, 2017 at 05:06:49PM +0200, Michal Privoznik wrote:
 On 08/07/2017 04:56 PM, Ján Tomko wrote:
> Make the comparison explicit.
> ---
>  src/conf/domain_conf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3cdb5e348..b5ce2ecd9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5359,10 +5359,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>  }
>  
>  if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
> -(info->rombar || info->romfile)) {
> +(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
>  
>  virBufferAddLit(buf, " -if (info->rombar) {
> +if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
>  const char *rombar = 
> virTristateSwitchTypeToString(info->rombar);
>  
>  if (rombar)
>

 I'm not against this patch, it's just that we set ABSENT explicitly to
 zero value so that we can do shortcuts like this. If we don't want to
 have them, we ought to remove the explicit value assignment.
>>>
>>> The shortcut is nice, but I don't like it personally.  If the variable
>>> can contain more than two states I'd rather check it explicitly. 
>>
>> So what's the point of assigning _ABSENT zero value then?
>>
>>> That's
>>> why I prefer (int == 0) over (!int).
>>
>> Well, if this is a part of bigger statement then yes, for instance:
>>
>> if (x == 0) {
>> } else if (x == 1) {
>> } else {
>> }
>>
>> (although, sometimes we might prefer switch() for that). But if it's
>> just a simple check whether a value was set or is equal to some default
>> (= if the check is interested in distinguishing just two states anyway),
>> !var works for me too:
>>
>> if (x)
>>formatToXML(x)
> 
> IMHO we should use the same form for all checks.
> 
>>
>> But sure, var == 0 vs !var is a personal preference. The important part
>> is my first question. If we dislike these shortcuts (in either of their
>> form), shouldn't we just drop explicit value assignment in the enum?
> 
> The point is that the value 0 is still named as _ABSENT.  The benefit
> of not using the shortcut for the VIR_TRISTATE_*_ABSENT is that you know
> right away what the variable stores and also you now right away that
> this part of code is executed only if the tristate was set to something.

This is argument for being explicit about checks (which I'm not
objecting to, I said at the beginning that I'm up for it). The thing I'm
interested in is: if we are explicit in all the places, what's the point
in having explicit assignment _ABSENT = 0? We have some enums without
explicit value assignment to any member. The only reason I can think of
is that when we alloc new struct, the memory chunk is prefilled with
zeroes so we don't have to do explicit setting to _ABSENT.

Michal

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

Re: [libvirt] support for configuring all ports of a multiport SRIOV VF when assigning to guest

[Moshe Levi]

Hi Laine,

Our Driver Architect  is on PTO, I am waiting for his return to provide answer 
to your questions 

-Original Message-
From: sendmail [mailto:justsendmailnothinge...@gmail.com] On Behalf Of Laine 
Stump
Sent: Saturday, August 5, 2017 8:14 PM
To: Libvirt 
Cc: Moshe Levi ; Doug Ledford ; 
Daniel P. Berrange 
Subject: Re: support for configuring all ports of a multiport SRIOV VF when 
assigning to guest

On 08/03/2017 02:33 AM, Moshe Levi wrote:
> Hi Laine,
>
> I have a few question before I can give my opinion.
> I the Mellanox Card Dual Port that support one PCI with 2 PF is  
> ConnectX-3 and ConnectX-3 Pro. (maybe others cards  I will check this) The 
> ConnectX-4 Dual Port and above is implemented with 2 PCI devices per 2 PF.

So is the "multiple netdevs for a single PCI device" hardware model completely 
deprecated, and will never show up again in new products?

If that's the case, maybe I shouldn't burden libvirt's config with all this new 
config that will only be used for legacy hardware. Perhaps a better approach 
would be to stick with the current config, and make it work properly for VFIO 
device assignment when a ConnectX-3 card is configured in single port mode - 
even that doesn't work correctly now[*] but it's a more easily solved problem, 
and can be done with no config changes.

Opinions about this? If it's a deadend and the existing legacy hardware can be 
used in a reasonable manner by setting it to single port mode, I don't want to 
add externally-visible knobs to libvirt.


[*]If a VF is configured to be "port 2 only", libvirt would still try to 
get/set the MAC and vlan tag with a netlink message to the *port 1* netdev of 
the PF, so it would be saving/setting the wrong (nonexistent?) VF netdev.

I did send patches yesterday (any reviews/testing appreciated!) that make 
everything work properly when saving/setting/restoring the VF netdev MAC/vlan 
tags on dual port cards used for macvtap passthrough:

  
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.redhat.com%2Farchives%2Flibvir-list%2F2017-August%2Fmsg00170.html=02%7C01%7Cmoshele%40mellanox.com%7Cb9e42d1d52484a36bae508d4dc257043%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636375500749886080=bLCKHyzJ%2BFkO4mWFYaSpdUGsWQHzvEXSFrhnpDS7kEQ%3D=0

Those don't fix the situation when doing VFIO device assignment, but they do at 
least make macvtap passthrough work correctly (for all VF netdevs, even when 
the netdevs are dual port!), and are the first step in getting it right for 
VFIO.


> I can check with our driver architect why it was done like this in the past.

They will obviously have much better insight than me :-), but my understanding 
from the outside is that one reason for doing it this way was to lessen the 
total amount of MMIO space usage on the host. Since each VF (PCI device) uses 
8MB or something of MMIO, which can really add up when you're talking about the 
difference between e.g. 64 VFs for 128 netdevs in dual port mode vs 128 VFs for 
128 netdevs in single port mode (but of course this assumes that it's 
okay/desirable to assign the netdevs to guests in pairs.


> The pci address in the xml below is VF pci which is different between all VF 
> so I am not sure why it causing problems with libvirt for setting mac? 

The problem is that in dual port mode, each VF has *2* netdevs associated with 
it. Each of those netdevs has its own MAC address, but is a part of the same 
PCI device. It would of course be possible to just add another MAC address to 
the  element, but I dislike that because I think of 
 as being "a single network device" and adding config for a 2nd 
network device is breaking that model. (i.e. it would work, but it looks ugly, 
and gets uglier when you add vlan tag to the mix)


> 
>   
> 
>
>
> I will do a little shift to openstack with SR-IOV mechanism driver.
> I remember with try to enable support on the second port for such  
> cards in openstack I remember we tested Mellanox ConnectX-3  Pro Dual Port  
> with openstack to allow boot a vm on both ports.
> I implemented the pci-passthrough-whitelist-regex  to allow a flexibly way to 
> whitelist pci device.
> And we also had a patch in neutron for the SR-IOV to allow the agent to allow 
> mapping of multiple PFs to a PCI device, but the community didn't like it 
> especially intel. 

Looking through that, it sounds like the person from Intel had never before 
heard of a card that put separate netdevs on the same PCI device, and he didn't 
really understand the concept. He was trying to see everything in the "1 netdev 
for 1 PCI address" model that he was used to, and in that framework what you 
were saying didn't make sense to him.

(for my part, I don't understand openstack code, so I don't really understand 
the details of what your patch was trying to do, but that's a separate / 
orthogonal problem; I *do* 

[libvirt] [PATCH v4 7/7] virsh: Implement managedsave-edit command

[Kothapally Madhu Pavan]

Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan 
---
 tools/virsh-domain.c | 72 
 tools/virsh.pod  | 21 +++
 2 files changed, 93 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ce15090..6152661 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4705,6 +4705,72 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "managedsave-edit" command
+ */
+static const vshCmdInfo info_managed_save_edit[] = {
+   {.name = "help",
+.data = N_("edit XML for a domain's managed save state file")
+   },
+   {.name = "desc",
+.data = N_("Edit the domain XML associated with the managed save state 
file")
+   },
+   {.name = NULL}
+};
+
+static const vshCmdOptDef opts_managed_save_edit[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL,
+{.name = "running",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be running on start")
+},
+{.name = "paused",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be paused on start")
+},
+{.name = NULL}
+};
+
+static bool
+cmdManagedSaveEdit(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+virDomainPtr dom = NULL;
+unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE;
+unsigned int define_flags = 0;
+
+if (vshCommandOptBool(cmd, "running"))
+define_flags |= VIR_DOMAIN_SAVE_RUNNING;
+if (vshCommandOptBool(cmd, "paused"))
+define_flags |= VIR_DOMAIN_SAVE_PAUSED;
+
+VSH_EXCLUSIVE_OPTIONS("running", "paused");
+
+dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+#define EDIT_GET_XML virDomainManagedSaveGetXMLDesc(dom, getxml_flags)
+#define EDIT_NOT_CHANGED   
   \
+do {   
   \
+vshPrintExtra(ctl, _("Managed save image of domain %s XML 
configuration " \
+ "not changed.\n"), virDomainGetName(dom));
   \
+ret = true;
   \
+goto edit_cleanup; 
   \
+} while (0)
+#define EDIT_DEFINE \
+(virDomainManagedSaveDefineXML(dom, doc_edited, define_flags) == 0)
+#include "virsh-edit.c"
+
+vshPrintExtra(ctl, _("Managed save image of Domain %s XML configuration 
edited.\n"),
+  virDomainGetName(dom));
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+return ret;
+}
+
+/*
  * "managedsave-dumpxml" command
  */
 static const vshCmdInfo info_managed_save_dumpxml[] = {
@@ -13968,6 +14034,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_managedsaveremove,
  .flags = 0
 },
+{.name = "managedsave-edit",
+ .handler = cmdManagedSaveEdit,
+ .opts = opts_managed_save_edit,
+ .info = info_managed_save_edit,
+ .flags = 0
+},
 {.name = "managedsave-dumpxml",
  .handler = cmdManagedSaveDumpxml,
  .opts = opts_managed_save_dumpxml,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1bda44f..dbad9a0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1650,6 +1650,27 @@ Extract the domain XML that was in effect at the time 
the saved state
 file I was created with the B command.  Using
 I<--security-info> will also include security sensitive information.
 
+=item B I [{I<--running> | I<--paused>}]
+
+Edit the XML configuration associated with a saved state file of a
+I was created by the B command.
+
+The managed save image records whether the domain should be started to a
+running or paused state.  Normally, this command does not alter the
+recorded state; passing either the I<--running> or I<--paused> flag
+will allow overriding which state the B should use.
+
+This is equivalent to:
+
+ virsh managedsave-dumpxml domain-name > state-file.xml
+ vi state-file.xml (or make changes with your other text editor)
+ virsh managedsave-define domain-name state-file-xml
+
+except that it does some error checking.
+
+The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
+variables, and defaults to C.
+
 =item B [I]
 
 Provide the maximum number of virtual CPUs supported for a guest VM on
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] conf: check rombar against VIR_DOMAIN_TRISTATE_SWITCH_ABSENT

[Pavel Hrdina]

On Mon, Aug 07, 2017 at 05:50:51PM +0200, Michal Privoznik wrote:
> On 08/07/2017 05:30 PM, Pavel Hrdina wrote:
> > On Mon, Aug 07, 2017 at 05:06:49PM +0200, Michal Privoznik wrote:
> >> On 08/07/2017 04:56 PM, Ján Tomko wrote:
> >>> Make the comparison explicit.
> >>> ---
> >>>  src/conf/domain_conf.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >>> index 3cdb5e348..b5ce2ecd9 100644
> >>> --- a/src/conf/domain_conf.c
> >>> +++ b/src/conf/domain_conf.c
> >>> @@ -5359,10 +5359,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
> >>>  }
> >>>  
> >>>  if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
> >>> -(info->rombar || info->romfile)) {
> >>> +(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
> >>>  
> >>>  virBufferAddLit(buf, " >>> -if (info->rombar) {
> >>> +if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
> >>>  const char *rombar = 
> >>> virTristateSwitchTypeToString(info->rombar);
> >>>  
> >>>  if (rombar)
> >>>
> >>
> >> I'm not against this patch, it's just that we set ABSENT explicitly to
> >> zero value so that we can do shortcuts like this. If we don't want to
> >> have them, we ought to remove the explicit value assignment.
> > 
> > The shortcut is nice, but I don't like it personally.  If the variable
> > can contain more than two states I'd rather check it explicitly. 
> 
> So what's the point of assigning _ABSENT zero value then?
> 
> > That's
> > why I prefer (int == 0) over (!int).
> 
> Well, if this is a part of bigger statement then yes, for instance:
> 
> if (x == 0) {
> } else if (x == 1) {
> } else {
> }
> 
> (although, sometimes we might prefer switch() for that). But if it's
> just a simple check whether a value was set or is equal to some default
> (= if the check is interested in distinguishing just two states anyway),
> !var works for me too:
> 
> if (x)
>formatToXML(x)

IMHO we should use the same form for all checks.

> 
> But sure, var == 0 vs !var is a personal preference. The important part
> is my first question. If we dislike these shortcuts (in either of their
> form), shouldn't we just drop explicit value assignment in the enum?

The point is that the value 0 is still named as _ABSENT.  The benefit
of not using the shortcut for the VIR_TRISTATE_*_ABSENT is that you know
right away what the variable stores and also you now right away that
this part of code is executed only if the tristate was set to something.

Yes, it this particular case you can see few lines below that there is
virTristateSwitchTypeToString() which gives you hint what the variable
is, but it's not always that way.

Pavel


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

[libvirt] [PATCH v4 2/7] lib: Add API to edit domain's managed save state xml configuration

[Kothapally Madhu Pavan]

Similar to domainSaveImageDefineXML this commit adds domainManagedSaveDefineXML
API which allows to edit domain's managed save state xml configuration.

Signed-off-by: Kothapally Madhu Pavan 
---
 include/libvirt/libvirt-domain.h |  4 +++
 src/driver-hypervisor.h  |  6 +
 src/libvirt-domain.c | 58 
 src/libvirt_public.syms  |  1 +
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 16 ++-
 src/remote_protocol-structs  |  8 +-
 7 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 8ede912..ed2c2b6 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1211,6 +1211,10 @@ int
virDomainManagedSaveRemove(virDomainPtr dom,
   unsigned int flags);
 char * virDomainManagedSaveGetXMLDesc(virDomainPtr domain,
   unsigned int flags);
+intvirDomainManagedSaveDefineXML(virDomainPtr domain,
+ const char *dxml,
+ unsigned int flags);
+
 
 /*
  * Domain core dump
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 598fc06..0a4181e 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -749,6 +749,11 @@ typedef char *
 (*virDrvDomainManagedSaveGetXMLDesc)(virDomainPtr domain,
  unsigned int flags);
 
+typedef int
+(*virDrvDomainManagedSaveDefineXML)(virDomainPtr domain,
+const char *dxml,
+unsigned int flags);
+
 typedef virDomainSnapshotPtr
 (*virDrvDomainSnapshotCreateXML)(virDomainPtr domain,
  const char *xmlDesc,
@@ -1427,6 +1432,7 @@ struct _virHypervisorDriver {
 virDrvDomainHasManagedSaveImage domainHasManagedSaveImage;
 virDrvDomainManagedSaveRemove domainManagedSaveRemove;
 virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc;
+virDrvDomainManagedSaveDefineXML domainManagedSaveDefineXML;
 virDrvDomainSnapshotCreateXML domainSnapshotCreateXML;
 virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc;
 virDrvDomainSnapshotNum domainSnapshotNum;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cb260e2..918e81a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9349,6 +9349,64 @@ virDomainManagedSaveGetXMLDesc(virDomainPtr domain, 
unsigned int flags)
 
 
 /**
+ * virDomainManagedSaveDefineXML:
+ * @domain: a domain object
+ * @dxml: XML config for adjusting guest xml used on restore
+ * @flags: bitwise-OR of virDomainSaveRestoreFlags
+ *
+ * This updates the definition of a domain stored in a saved state
+ * file. @domain is used to extract the saved state file location.
+ *
+ * @dxml can be used to alter host-specific portions of the domain XML
+ * that will be used on the next start of the domain. For example, it is
+ * possible to alter the backing filename that is associated with a
+ * disk device, to match renaming done as part of backing up the disk
+ * device while the domain is stopped.
+ *
+ * Normally, the saved state file will remember whether the domain was
+ * running or paused, and restore defaults to the same state.
+ * Specifying VIR_DOMAIN_SAVE_RUNNING or VIR_DOMAIN_SAVE_PAUSED in
+ * @flags will override the default saved into the file; omitting both
+ * leaves the file's default unchanged.  These two flags are mutually
+ * exclusive.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virDomainManagedSaveDefineXML(virDomainPtr domain, const char *dxml,
+  unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "flags=%x", flags);
+
+virResetLastError();
+
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+
+if (conn->driver->domainManagedSaveDefineXML) {
+int ret;
+ret = conn->driver->domainManagedSaveDefineXML(domain, dxml, flags);
+
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return -1;
+}
+
+
+/**
  * virDomainOpenConsole:
  * @dom: a domain object
  * @dev_name: the console, serial or parallel port device alias, or NULL
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index b38d0bb..fc4efd9 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -771,6 +771,7 @@ LIBVIRT_3.4.0 {
 LIBVIRT_3.7.0 {
 global:
 virDomainManagedSaveGetXMLDesc;
+   

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Kirti Wankhede]

On 8/7/2017 1:11 PM, Gao, Ping A wrote:
> 
> On 2017/8/4 5:11, Alex Williamson wrote:
>> On Thu, 3 Aug 2017 20:26:14 +0800
>> "Gao, Ping A"  wrote:
>>
>>> On 2017/8/3 0:58, Alex Williamson wrote:
 On Wed, 2 Aug 2017 21:16:28 +0530
 Kirti Wankhede  wrote:
  
> On 8/2/2017 6:29 PM, Gao, Ping A wrote:  
>> On 2017/8/2 18:19, Kirti Wankhede wrote:
>>> On 8/2/2017 3:56 AM, Alex Williamson wrote:
 On Tue, 1 Aug 2017 13:54:27 +0800
 "Gao, Ping A"  wrote:

> On 2017/7/28 0:00, Gao, Ping A wrote:
>> On 2017/7/27 0:43, Alex Williamson wrote:  
>>> [cc +libvir-list]
>>>
>>> On Wed, 26 Jul 2017 21:16:59 +0800
>>> "Gao, Ping A"  wrote:
>>>  
 The vfio-mdev provide the capability to let different guest share 
 the
 same physical device through mediate sharing, as result it bring a
 requirement about how to control the device sharing, we need a QoS
 related interface for mdev to management virtual device resource.

 E.g. In practical use, vGPUs assigned to different quests almost 
 has
 different performance requirements, some guests may need higher 
 priority
 for real time usage, some other may need more portion of the GPU
 resource to get higher 3D performance, corresponding we can define 
 some
 interfaces like weight/cap for overall budget control, priority for
 single submission control.

 So I suggest to add some common attributes which are vendor 
 agnostic in
 mdev core sysfs for QoS purpose.  
>>> I think what you're asking for is just some standardization of a QoS
>>> attribute_group which a vendor can optionally include within the
>>> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
>>> transparently enable this, but it really only provides the standard,
>>> all of the support code is left for the vendor.  I'm fine with that,
>>> but of course the trouble with and sort of standardization is 
>>> arriving
>>> at an agreed upon standard.  Are there QoS knobs that are generic
>>> across any mdev device type?  Are there others that are more 
>>> specific
>>> to vGPU?  Are there existing examples of this that we can steal 
>>> their
>>> specification?  
>> Yes, you are right, standardization QoS knobs are exactly what I 
>> wanted.
>> Only when it become a part of the mdev framework and libvirt, then 
>> QoS
>> such critical feature can be leveraged by cloud usage. HW vendor only
>> need to focus on the implementation of the corresponding QoS 
>> algorithm
>> in their back-end driver.
>>
>> Vfio-mdev framework provide the capability to share the device that 
>> lack
>> of HW virtualization support to guests, no matter the device type,
>> mediated sharing actually is a time sharing multiplex method, from 
>> this
>> point of view, QoS can be take as a generic way about how to control 
>> the
>> time assignment for virtual mdev device that occupy HW. As result we 
>> can
>> define QoS knob generic across any device type by this way. Even if 
>> HW
>> has build in with some kind of QoS support, I think it's not a 
>> problem
>> for back-end driver to convert mdev standard QoS definition to their
>> specification to reach the same performance expectation. Seems there 
>> are
>> no examples for us to follow, we need define it from scratch.
>>
>> I proposal universal QoS control interfaces like below:
>>
>> Cap: The cap limits the maximum percentage of time a mdev device can 
>> own
>> physical device. e.g. cap=60, means mdev device cannot take over 60% 
>> of
>> total physical resource.
>>
>> Weight: The weight define proportional control of the mdev device
>> resource between guests, it’s orthogonal with Cap, to target load
>> balancing. E.g. if guest 1 should take double mdev device resource
>> compare with guest 2, need set weight ratio to 2:1.
>>
>> Priority: The guest who has higher priority will get execution first,
>> target to some real time usage and speeding interactive response.
>>
>> Above QoS interfaces cover both overall budget control and single
>> submission control. I will sent out detail design later once get 
>> aligned.  
> Hi Alex,
> Any comments about the