[libvirt] libvirt question

2017-05-18 Thread zhun...@gmail.com 
Hello，I wrote a program using libvirt API to get vm information like this:
/*dom  is virDomainPtr type*/
dom=virDomainLookupByID(conn,activeDomains[i]);
if(dom!=NULL)
printf("%d--%s\n",activeDomains[i],dom->name);
..
but when compile it,error occured like this:
vm_eraser_detect.c:125:54: error: dereferencing pointer to incomplete type
printf("%d--%s\n",activeDomains[i],dom->name);
what is the reason of it???
thanka a lot!


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

Re: [libvirt] [PATCH] datatypes: removing unnecessary return statement.

2017-05-18 Thread John Ferlan 


On 05/17/2017 09:53 PM, Julio Faracco wrote:
> There is a wrong 'return' statement after a 'goto' statement inside the
> function virConnectCloseCallbackDataRegister(). This commit only removes
> the 'return'.
> 
> Signed-off-by: Julio Faracco 
> ---
>  src/datatypes.c | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: John Ferlan 

(and pushed)

Tks -

John

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

Re: [libvirt] [PATCH] qemu: allow to control host side link status of network device

2017-05-18 Thread Vasiliy Tolstov 
2017-05-17 15:28 GMT+03:00 Peter Krempa :
>>
>> Well, *I* think I've given sufficient reasons for having the two link
>> states controlled separately, but since Dan and Peter had questioned its
>> usefulness, we should see whether or not I've swayed their opinions :-)
>
> I think we should have two elements in this case. I just don't consider
> setting the host link side to be very useful.
>

Sorry, Laine. As i understand you already have alternative patch for
such feature, can you share it?
I'm really interesting how much it different from my.



-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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

Re: [libvirt] [PATCH 7/8] appmor, libvirt-qemu: Add 9p support

2017-05-18 Thread Serge E. Hallyn 
Quoting Guido Günther (a...@sigxcpu.org):
> On Thu, May 18, 2017 at 11:21:54AM -0500, Serge E. Hallyn wrote:
> > Mind you I'm not crazy about this.  If this could be toggled with a
> > default-off config option that would seem better than always giving
> > these caps to libvirt-qemu.
> 
> virt-aa-helper could add these if it detects a 9pfs file system. That
> would be better than always adding it.

Agreed

> Cheers,
>  -- Guido
> 
> > 
> > Quoting Stefan Bader (stefan.ba...@canonical.com):
> > > From: Serge Hallyn 
> > > 
> > > Add fowner and fsetid to libvirt-qemu profile.
> > > 
> > > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
> > > 
> > > Signed-off-by: Christian Ehrhardt 
> > > Signed-off-by: Stefan Bader 
> > > ---
> > >  examples/apparmor/libvirt-qemu | 4 
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/examples/apparmor/libvirt-qemu 
> > > b/examples/apparmor/libvirt-qemu
> > > index 89466c9..f04ce04 100644
> > > --- a/examples/apparmor/libvirt-qemu
> > > +++ b/examples/apparmor/libvirt-qemu
> > > @@ -13,6 +13,10 @@
> > >capability setgid,
> > >capability setuid,
> > >  
> > > +  # for 9p
> > > +  capability fsetid,
> > > +  capability fowner,
> > > +
> > >network inet stream,
> > >network inet6 stream,
> > >  
> > > -- 
> > > 2.7.4
> > 

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

Re: [libvirt] [PATCH 7/8] appmor, libvirt-qemu: Add 9p support

2017-05-18 Thread Guido Günther 
On Thu, May 18, 2017 at 11:21:54AM -0500, Serge E. Hallyn wrote:
> Mind you I'm not crazy about this.  If this could be toggled with a
> default-off config option that would seem better than always giving
> these caps to libvirt-qemu.

virt-aa-helper could add these if it detects a 9pfs file system. That
would be better than always adding it.
Cheers,
 -- Guido

> 
> Quoting Stefan Bader (stefan.ba...@canonical.com):
> > From: Serge Hallyn 
> > 
> > Add fowner and fsetid to libvirt-qemu profile.
> > 
> > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
> > 
> > Signed-off-by: Christian Ehrhardt 
> > Signed-off-by: Stefan Bader 
> > ---
> >  examples/apparmor/libvirt-qemu | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> > index 89466c9..f04ce04 100644
> > --- a/examples/apparmor/libvirt-qemu
> > +++ b/examples/apparmor/libvirt-qemu
> > @@ -13,6 +13,10 @@
> >capability setgid,
> >capability setuid,
> >  
> > +  # for 9p
> > +  capability fsetid,
> > +  capability fowner,
> > +
> >network inet stream,
> >network inet6 stream,
> >  
> > -- 
> > 2.7.4
> 

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

Re: [libvirt] [PATCH 7/8] appmor, libvirt-qemu: Add 9p support

2017-05-18 Thread Serge E. Hallyn 
Mind you I'm not crazy about this.  If this could be toggled with a
default-off config option that would seem better than always giving
these caps to libvirt-qemu.

Quoting Stefan Bader (stefan.ba...@canonical.com):
> From: Serge Hallyn 
> 
> Add fowner and fsetid to libvirt-qemu profile.
> 
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/libvirt-qemu | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index 89466c9..f04ce04 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -13,6 +13,10 @@
>capability setgid,
>capability setuid,
>  
> +  # for 9p
> +  capability fsetid,
> +  capability fowner,
> +
>network inet stream,
>network inet6 stream,
>  
> -- 
> 2.7.4

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

Re: [libvirt] Various apparmor related changes (part 1), version 2

2017-05-18 Thread Serge E. Hallyn 
Quoting Stefan Bader (stefan.ba...@canonical.com):
> > Over the years there have been a bunch of changes to the
> > apparmor profiles and/or virt-aa-helper which have been
> > carried in Debian/Ubuntu but never made it upstream.
> > 
> > In an attempt to clean this up and generally improve the
> > apparmor based environments, we (Christian and I) went
> > over the changes, cleaned out cruft as much as possible 
> > and would be sending out hunks of changes to this list
> > for upstream inclusion.
> > 
> > I hope doing multiple but smaller rounds of submissions
> > will make it simpler to get those reviewed and hopefully
> > accepted.
> 
> For the second version I added acks, merged the patches
> related to explicit device denials and local apparmor
> profiles, and split the 9p support one (holding back the
> part allowing link access for later or to be replaced by
> a safer solution).
> I also tried to improve the explanation in the description
> of patch #1 (virt-aa-helper: Ask for no deny rule for readonly
> disk elements).
> 
> Thanks,
> Stefan

Thanks,

Acked-by: Serge Hallyn 

I don't like the added capabilities in the one patch, but I'm not
nacking it on that account.  Still a toggle would be comforting.
Make people who want 9p consciously sign in to the added privs.

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

[libvirt] [PATCH 1/3] virfile: Provide stub for virFileInData

2017-05-18 Thread Michal Privoznik 
Some older systems (such as RHEL6) lack SEEK_HOLE and SEEK_DATA
which virFileInData relies on. Provide a stub for these systems.

Signed-off-by: Michal Privoznik 
---
 configure.ac   |  5 +
 src/util/virfile.c | 15 +++
 2 files changed, 20 insertions(+)

diff --git a/configure.ac b/configure.ac
index f20b9ea4d..2e6051354 100644
--- a/configure.ac
+++ b/configure.ac
@@ -352,6 +352,11 @@ AC_CHECK_DECLS([ETH_FLAG_TXVLAN, ETH_FLAG_NTUPLE, 
ETH_FLAG_RXHASH, ETH_FLAG_LRO,
   [], [], [[#include 
   ]])
 
+AC_CHECK_DECLS([SEEK_HOLE], [], [],
+   [#include 
+#include ])
+
+
 dnl Our only use of libtasn1.h is in the testsuite, and can be skipped
 dnl if the header is not present.  Assume -ltasn1 is present if the
 dnl header could be found.
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 2f4bc42b6..b7645fbfc 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3798,6 +3798,7 @@ virFileComparePaths(const char *p1, const char *p2)
 }
 
 
+#if HAVE_DECL_SEEK_HOLE
 /**
  * virFileInData:
  * @fd: file to check
@@ -3904,6 +3905,20 @@ virFileInData(int fd,
 return ret;
 }
 
+#else /* !HAVE_DECL_SEEK_HOLE */
+
+int
+virFileInData(int fd ATTRIBUTE_UNUSED,
+  int *inData ATTRIBUTE_UNUSED,
+  long long *length ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, "%s",
+ _("sparse files not supported"));
+return -1;
+}
+
+#endif /* !HAVE_DECL_SEEK_HOLE */
+
 
 /**
  * virFileReadValueInt:
-- 
2.13.0

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

[libvirt] [PATCH 0/3] Couple of build fixes after sparse streams

2017-05-18 Thread Michal Privoznik 
Some systems lack the symbols we need or have them in different header files.

Michal Privoznik (3):
  virfile: Provide stub for virFileInData
  virfiletest: Test virFileInData iff SEEK_HOLE is defined
  virfiletest: include linux/falloc.h

 configure.ac|  5 +
 src/util/virfile.c  | 15 +++
 tests/virfiletest.c | 40 +++-
 3 files changed, 47 insertions(+), 13 deletions(-)

-- 
2.13.0

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

[libvirt] [PATCH 2/3] virfiletest: Test virFileInData iff SEEK_HOLE is defined

2017-05-18 Thread Michal Privoznik 
Yet another place where we need to wrap code in
HAVE_DECL_SEEK_HOLE block.

Signed-off-by: Michal Privoznik 
---
 tests/virfiletest.c | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/tests/virfiletest.c b/tests/virfiletest.c
index a93bee01a..d6a526c13 100644
--- a/tests/virfiletest.c
+++ b/tests/virfiletest.c
@@ -123,7 +123,11 @@ static int
 makeSparseFile(const off_t offsets[],
const bool startData);
 
-#ifdef __linux__
+static bool
+holesSupported(void);
+
+#if HAVE_DECL_SEEK_HOLE
+
 /* Create a sparse file. @offsets in KiB. */
 static int
 makeSparseFile(const off_t offsets[],
@@ -188,19 +192,8 @@ makeSparseFile(const off_t offsets[],
 return -1;
 }
 
-#else /* !__linux__ */
 
-static int
-makeSparseFile(const off_t offsets[] ATTRIBUTE_UNUSED,
-   const bool startData ATTRIBUTE_UNUSED)
-{
-return -1;
-}
-
-#endif /* !__linux__ */
-
-
-#define EXTENT 4
+# define EXTENT 4
 static bool
 holesSupported(void)
 {
@@ -245,6 +238,23 @@ holesSupported(void)
 return ret;
 }
 
+#else /* !HAVE_DECL_SEEK_HOLE */
+
+static int
+makeSparseFile(const off_t offsets[] ATTRIBUTE_UNUSED,
+   const bool startData ATTRIBUTE_UNUSED)
+{
+return -1;
+}
+
+
+static bool
+holesSupported(void)
+{
+return false;
+}
+
+#endif /* !HAVE_DECL_SEEK_HOLE */
 
 struct testFileInData {
 bool startData; /* whether the list of offsets starts with data 
section */
-- 
2.13.0

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

[libvirt] [PATCH 3/3] virfiletest: include linux/falloc.h

2017-05-18 Thread Michal Privoznik 
On systems with older glibc including fcntl.h for getting
FALLOC_FL_PUNCH_HOLE defined is not enough. We must also include
linux/falloc.h.

Signed-off-by: Michal Privoznik 
---
 tests/virfiletest.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/virfiletest.c b/tests/virfiletest.c
index d6a526c13..d5bded00e 100644
--- a/tests/virfiletest.c
+++ b/tests/virfiletest.c
@@ -27,6 +27,10 @@
 #include "virfile.h"
 #include "virstring.h"
 
+#ifdef __linux__
+# include 
+#endif
+
 
 #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
 static int testFileCheckMounts(const char *prefix,
-- 
2.13.0

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

[libvirt] [PATCH] nodedev: mdev: Fix build caused by symbol shadowing

2017-05-18 Thread Erik Skultety 
GCC 4.6 complains about a local declaration shadowing a global symbol.

Signed-off-by: Erik Skultety 
---
Pushed as build breaker.

 src/node_device/node_device_udev.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 8288be1cb..4ecb0b18f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1076,16 +1076,16 @@ udevProcessMediatedDevice(struct udev_device *dev,
 const char *uuidstr = NULL;
 int iommugrp = -1;
 char *linkpath = NULL;
-char *realpath = NULL;
+char *canonicalpath = NULL;
 virNodeDevCapMdevPtr data = >caps->data.mdev;

 if (virAsprintf(, "%s/mdev_type", udev_device_get_syspath(dev)) < 
0)
 goto cleanup;

-if (virFileResolveLink(linkpath, ) < 0)
+if (virFileResolveLink(linkpath, ) < 0)
 goto cleanup;

-if (VIR_STRDUP(data->type, last_component(realpath)) < 0)
+if (VIR_STRDUP(data->type, last_component(canonicalpath)) < 0)
 goto cleanup;

 uuidstr = udev_device_get_sysname(dev);
@@ -1100,7 +1100,7 @@ udevProcessMediatedDevice(struct udev_device *dev,
 ret = 0;
  cleanup:
 VIR_FREE(linkpath);
-VIR_FREE(realpath);
+VIR_FREE(canonicalpath);
 return ret;
 }

--
2.13.0

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

[libvirt] [PATCH] cpu_ppc64: Add support for host-model on POWER9

2017-05-18 Thread Jiri Denemark 
Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_ppc64.c|  8 
 .../qemuxml2argv-pseries-cpu-compat-power9.args| 24 ++
 .../qemuxml2argv-pseries-cpu-compat-power9.xml | 21 +++
 tests/qemuxml2argvtest.c   |  7 +++
 tests/testutilsqemu.c  | 13 +++-
 tests/testutilsqemu.h  |  1 +
 6 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index f64592b55..bf0859904 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -92,22 +92,22 @@ ppc64CheckCompatibilityMode(const char *host_model,
 if (!compat_mode)
 return VIR_CPU_COMPARE_IDENTICAL;
 
-/* Valid host CPUs: POWER6, POWER7, POWER8 */
+/* Valid host CPUs: POWER6, POWER7, POWER8, POWER9 */
 if (!STRPREFIX(host_model, "POWER") ||
 !(tmp = (char *) host_model + strlen("POWER")) ||
 virStrToLong_i(tmp, NULL, 10, ) < 0 ||
-host < 6 || host > 8) {
+host < 6 || host > 9) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
_("Host CPU does not support compatibility modes"));
 goto out;
 }
 
-/* Valid compatibility modes: power6, power7, power8 */
+/* Valid compatibility modes: power6, power7, power8, power9 */
 if (!STRPREFIX(compat_mode, "power") ||
 !(tmp = (char *) compat_mode + strlen("power")) ||
 virStrToLong_i(tmp, NULL, 10, ) < 0 ||
-compat < 6 || compat > 8) {
+compat < 6 || compat > 9) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown compatibility mode %s"),
compat_mode);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args 
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args
new file mode 100644
index 0..af93d63dc
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name QEMUGuest1 \
+-S \
+-M pseries \
+-cpu host,compat=power9 \
+-m 256 \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-boot c \
+-usb \
+-chardev pty,id=charserial0 \
+-device spapr-vty,chardev=charserial0,reg=0x3000
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml
new file mode 100644
index 0..30ab5c267
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml
@@ -0,0 +1,21 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  4
+  
+hvm
+  
+  
+power9
+  
+  
+  
+  /usr/bin/qemu-system-ppc64
+  
+
+  
+  
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 426959857..669caa0e4 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1673,6 +1673,13 @@ mymain(void)
 QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
 DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM,
 QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
+DO_TEST_FAILURE("pseries-cpu-compat-power9", QEMU_CAPS_KVM);
+
+qemuTestSetHostCPU(driver.caps, cpuPower9);
+DO_TEST("pseries-cpu-compat-power9",
+QEMU_CAPS_KVM, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
+qemuTestSetHostCPU(driver.caps, NULL);
+
 qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
 
 DO_TEST("pseries-panic-missing",
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 709e291bd..ee4853841 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -17,6 +17,7 @@
 virCPUDefPtr cpuDefault;
 virCPUDefPtr cpuHaswell;
 virCPUDefPtr cpuPower8;
+virCPUDefPtr cpuPower9;
 
 static virCPUFeatureDef cpuDefaultFeatures[] = {
 { (char *) "ds",-1 },
@@ -94,6 +95,15 @@ static virCPUDef cpuPower8Data = {
 .threads = 8,
 };
 
+static virCPUDef cpuPower9Data = {
+.type = VIR_CPU_TYPE_HOST,
+.arch = VIR_ARCH_PPC64,
+.model = (char *) "POWER9",
+.sockets = 1,
+.cores = 16,
+.threads = 1,
+};
+
 typedef enum {
 TEST_UTILS_QEMU_BIN_I686,
 TEST_UTILS_QEMU_BIN_X86_64,
@@ -467,7 +477,8 @@ virCapsPtr testQemuCapsInit(void)
 
 if (!(cpuDefault = virCPUDefCopy()) ||
 !(cpuHaswell = virCPUDefCopy()) ||
-!(cpuPower8 =

Re: [libvirt] [PATCH] qemu: monitor: Don't bother extracting vCPU halted state in text monitor

2017-05-18 Thread Peter Krempa 
On Thu, May 18, 2017 at 13:53:55 +0200, Ján Tomko wrote:
> On Thu, May 18, 2017 at 01:47:03PM +0200, Peter Krempa wrote:
> > The code causes the 'offset' variable to be overwritten (possibly with
> > NULL if neither of the vCPUs is halted) which causes a crash since the
> > variable is still used after that part.
> > 
> > Additionally there's a bug, since strstr() would look up the '(halted)'
> > string in the whole string rather than just the currently processed line
> > the returned data is completely bogus.
> > 
> > Rather than switching to single line parsing let's remove the code
> > altogether since it has a commonly used JSON monitor alternative and
> > the data itself is not very useful to report.
> > 
> > The code was introduced in commit cc5e695bde
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452106
> > ---
> > src/qemu/qemu_monitor_text.c | 6 --
> > 1 file changed, 6 deletions(-)
> > 
> 
> ACK
> 
> When will we able to kill the whole file?

Once we finaly decide, that centos 6.0 + upstream libvirt is an insane
configuration.


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

Re: [libvirt] [PATCH 0/2] Couple of sparse streams improvements

2017-05-18 Thread Ján Tomko 

On Thu, May 18, 2017 at 08:53:34AM +0200, Michal Privoznik wrote:

News entry & one simple fix.

Michal Privoznik (2):
 news: Document sparse streams
 virStream: Forbid negative seeks

docs/news.xml| 10 ++
src/internal.h   |  7 +++
src/rpc/virnetclientstream.c |  1 +
src/util/virfdstream.c   |  1 +
4 files changed, 19 insertions(+)



ACK series

Jan


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

Re: [libvirt] [PATCH] qemu: monitor: Don't bother extracting vCPU halted state in text monitor

2017-05-18 Thread Ján Tomko 

On Thu, May 18, 2017 at 01:47:03PM +0200, Peter Krempa wrote:

The code causes the 'offset' variable to be overwritten (possibly with
NULL if neither of the vCPUs is halted) which causes a crash since the
variable is still used after that part.

Additionally there's a bug, since strstr() would look up the '(halted)'
string in the whole string rather than just the currently processed line
the returned data is completely bogus.

Rather than switching to single line parsing let's remove the code
altogether since it has a commonly used JSON monitor alternative and
the data itself is not very useful to report.

The code was introduced in commit cc5e695bde

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452106
---
src/qemu/qemu_monitor_text.c | 6 --
1 file changed, 6 deletions(-)



ACK

When will we able to kill the whole file?

Jan


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

[libvirt] [PATCH] qemu: monitor: Don't bother extracting vCPU halted state in text monitor

2017-05-18 Thread Peter Krempa 
The code causes the 'offset' variable to be overwritten (possibly with
NULL if neither of the vCPUs is halted) which causes a crash since the
variable is still used after that part.

Additionally there's a bug, since strstr() would look up the '(halted)'
string in the whole string rather than just the currently processed line
the returned data is completely bogus.

Rather than switching to single line parsing let's remove the code
altogether since it has a commonly used JSON monitor alternative and
the data itself is not very useful to report.

The code was introduced in commit cc5e695bde

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452106
---
 src/qemu/qemu_monitor_text.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 9c9eeea01..66c94fbcd 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -552,12 +552,6 @@ qemuMonitorTextQueryCPUs(qemuMonitorPtr mon,
 cpu.qemu_id = cpuid;
 cpu.tid = tid;

-/* Extract halted indicator */
-if ((offset = strstr(line, "(halted)")) != NULL)
-cpu.halted = true;
-else
-cpu.halted = false;
-
 if (VIR_APPEND_ELEMENT_COPY(cpus, ncpus, cpu) < 0) {
 ret = -1;
 goto cleanup;
-- 
2.12.2

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

Re: [libvirt] [PATCH v2 2/2] util: do not leak handles in stop netlink event service

2017-05-18 Thread John Ferlan 


On 05/18/2017 12:06 AM, Wang King wrote:
> handles stored in virNetlinkEventSrvPrivatePtr should be freed when
> stop netlink event service.

Altered the commit message to describe which commit introduced and just
fit that into the text...

> ---
>  src/util/virnetlink.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: John Ferlan 

Will push soon - tks,

John

> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index fad1e8e..7988312 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -758,6 +758,7 @@ virNetlinkEventServiceStop(unsigned int protocol)
>  }
>  
>  server[protocol] = NULL;
> +VIR_FREE(srv->handles);
>  virNetlinkEventServerUnlock(srv);
>  
>  virMutexDestroy(>lock);
> 

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

Re: [libvirt] [RFC] Fixing a regression caused by recent CPU driver changes

2017-05-18 Thread Martin Kletzander 

On Thu, May 18, 2017 at 11:14:50AM +0200, Pavel Hrdina wrote:

On Thu, May 18, 2017 at 10:22:59AM +0200, Jiri Denemark wrote:

Hi all,

when I was enhancing libvirt's guest CPU configuration code to be able
to really ensure stable guest CPU ABI, I added a new attribute
//cpu/@check which is nicely backward compatible... an old libvirt will
just ignore it. However, even if check='full' will be ignored, an old
libvirt will still see the updated CPU definition (features added or
removed by the hypervisor will be shown there). And because we need QEMU
2.9.0 to check what features are going to be added or removed before we
actually start the domain, migrating such domain to an older libvirt or
QEMU may fail if QEMU enables a feature which is not supported by the
host CPU. Known features causing problems are, e.g., x2apic, hypervisor,
and arat. To make things even worse, updating a CPU definition with the
automatically added/removed features can be done since QEMU 1.5.0.

Even save/restore or snapshot revert on a single host running new
libvirt and QEMU < 2.9.0 is now affected by this regression.

The big question is how to fix the regression in a backward compatible
way and still keep the ability to properly check guest CPU ABI with new
enough libvirt and QEMU. Clearly, we need to keep both the original and
the updated CPU definition (or one of them and a diff against the
other).

I came up with the following options:

1. When migrating a domain, the domain XML would contain the original
   CPU def while the updated one would be transferred in a migration
   cookie.
   - doesn't fix save/restore or snapshot revert
 - could be fixed by not updating the CPU with QEMU < 2.9.0, but it
   won't work when restore is done on a different host with older
   QEMU (and yes, this happens in real life, e.g., with oVirt)
   - doesn't even fix migration after save/restore or snapshot revert


This is clearly not good enough.


2. Use migration cookie and change the save image format to contain
   additional data (something like migration cookie) which a new libvirt
   could make use of while old libvirt would ignore any additional data
   it doesn't know about
   - snapshot XML would need to be updated too, but that's trivial
   - cleanly changing save image format requires its version to be
 increased and old libvirt will just refuse to read such image
   - this would fix save/restore on the same host or on a host with
 older QEMU
   - doesn't fix restore on a different host running older libvirt
 - even this is done by oVirt


This would be the best solution, however we might need to go with
option 3.  The CPU updating is a new feature but it's enabled
automatically without any way how to disable it and in this case we
should not break anything.

Changing the save image format by introducing new feature to
seve/restore APIs is valid reason to increase a version so old libvirt
will fail if the new feature of save/restore is used.  Unfortunately
this is not the case, we need to update the save image format to make
different feature backward compatible which should not break the save
image compatibility.


3. Use migration cookie and change the save image format without
   increasing its version (and update snapshot XML)
   - this fixes all cases
   - technically possible by using one of the unused 32b ints and
 adding \0 at the end of the domain XML followed by anothe XML with
 the additional data (pointed to by the formerly unused uint32_t)
   - very ugly hack


It's not that ugly.  We just need to workaround a bad design used to
store domain XML in the save image.  Currently there is no easy and
nice way how to add additional data into the XML without breaking
backward compatibility and using \0 is the only backward compatible way
of doing it.  Luckily we have another 15 unused 32b ints :).

The ideal solution would be to use proper XML:


 
   ...
 
 
   ...
 
 
   ...
 
 ...


but since we store the domain XML directly as binary data the only way
how to store additional data for backward compatible features we need to
"extend" the current binary data with additional XML:


 ...

\0

 
   ...
 
 
   ...
 
 ...


and the newly used 32b int will just point to the beginning of the
additional data.



Actually, one bit is enough.  It would just say that there are newer
save image data after the first one.  It could be daisy chained as the
next one would just have that field as well.

I'm ambivalent between 3 and 4.


4. Format both CPUs in domain XML by adding a new subelement in 
   which would list all extra features disabled or enabled by QEMU
   - fixes all cases without the need to use migration cookie, change
 save image format, or snapshot XML
   - but it may be very confusing for users and it would need to be
 documented as "output only, don't touch staff"


I don't like it, this just puts an extra noise into the XML that is
irrelevant for users.


5. Add the additional data in

Re: [libvirt] [PATCH v2 1/2] util: Deduplicate code in virNetlinkEventServiceStopAll

2017-05-18 Thread John Ferlan 


On 05/18/2017 12:06 AM, Wang King wrote:
> Commit 15a71e60 introduced the virNetlinkEventServiceStopAll function, and
> the code in virNetlinkEventServiceStop is copied to this function. can use
> virNetlinkEventServiceStop instead.
> ---
>  src/util/virnetlink.c | 25 +++--
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index 92ecf77..fad1e8e 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -775,32 +775,13 @@ virNetlinkEventServiceStop(unsigned int protocol)
>  int
>  virNetlinkEventServiceStopAll(void)
>  {
> -size_t i, j;
> +size_t i;
>  virNetlinkEventSrvPrivatePtr srv = NULL;

^^^
This wasn't necessary either...  I'll remove it before pushing.

Reviewed-by: John Ferlan 

John

>  
>  VIR_INFO("stopping all netlink event services");
>  
> -for (i = 0; i < MAX_LINKS; i++) {
> -srv = server[i];
> -if (!srv)
> -continue;
> -
> -virNetlinkEventServerLock(srv);
> -nl_close(srv->netlinknh);
> -virNetlinkFree(srv->netlinknh);
> -virEventRemoveHandle(srv->eventwatch);
> -
> -for (j = 0; j < srv->handlesCount; j++) {
> -if (srv->handles[j].deleted == VIR_NETLINK_HANDLE_VALID)
> -virNetlinkEventRemoveClientPrimitive(j, i);
> -}
> -
> -server[i] = NULL;
> -virNetlinkEventServerUnlock(srv);
> -
> -virMutexDestroy(>lock);
> -VIR_FREE(srv);
> -}
> +for (i = 0; i < MAX_LINKS; i++)
> +virNetlinkEventServiceStop(i);
>  
>  return 0;
>  }
> 

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

Re: [libvirt] [PATCH v4 4/6] nodedev: Introduce the mdev capability to a PCI parent device

2017-05-18 Thread Erik Skultety 
On Thu, May 18, 2017 at 06:48:48AM -0400, John Ferlan wrote:
> [...]
>
> >>> +static int
> >>> +udevFillMdevType(struct udev_device *device,
> >>> + const char *dir,
> >>> + virNodeDevCapMdevTypePtr type)
> >>> +{
> >>> +int ret = -1;
> >>> +char *attrpath = NULL;
> >>> +
> >>> +#define MDEV_GET_SYSFS_ATTR(attr_name, cb, ...)  
> >>>\
> >>> +do { 
> >>>\
> >>> +if (virAsprintf(, "%s/%s", dir, #attr_name) < 0)
> >>>\
> >>> +goto cleanup;
> >>>\
> >>> + 
> >>>\
> >>> +if (cb(device, attrpath, __VA_ARGS__) < 0)   
> >>>\
> >>> +goto cleanup;
> >>>\
> >>> + 
> >>>\
> >>> +VIR_FREE(attrpath);  
> >>>\
> >>> +} while (0)  
> >>>\
> >>> +
> >>> +if (VIR_STRDUP(type->id, last_component(dir)) < 0)
> >>> +goto cleanup;
> >>> +
> >>> +/* query udev for the attributes under subdirectories using the 
> >>> relative
> >>> + * path stored in @dir, i.e. 'mdev_supported_types/'
> >>> + */
> >>> +MDEV_GET_SYSFS_ATTR(name, udevGetStringSysfsAttr, >name);
> >>
> >> Does this imply that name isn't necessarily optional as defined in RNG?
> >> Can '@name' not exist and if it is optional how does that adjust the macro?
> >
> > There's no need for the macro to be changed - type->name == NULL in that 
> > case
> > which means it won't be formatted to the XML, there's no issue in that the
> > NULL's going to be handled gracefully all the way down from
> > udevGetStringSysfsAttr.
> >
>
> O
> >>
> >>> +MDEV_GET_SYSFS_ATTR(device_api, udevGetStringSysfsAttr, 
> >>> >device_api);
> >>> +MDEV_GET_SYSFS_ATTR(available_instances, udevGetUintSysfsAttr,
> >>> +>available_instances, 10);
> >>> +
> >>> +#undef MDEV_GET_SYSFS_ATTR
> >>> +
> >>> +ret = 0;
> >>> + cleanup:
> >>> +VIR_FREE(attrpath);
> >>> +return ret;
> >>> +}
> >>> +
> >>
> >>
> >> With changes to
> >>
> >>  1. fix the Coverity issues
> >>  2. determine whether the virNodeDeviceObjHasCap change is needed
> >>  3. address the optional name
> > ^ see my comment above
> >
> >>
> >> You have my:
> >>
> >> Reviewed-by: John Ferlan 
> >>
> >> While I'm not a fan of 'deviceAPI'
> >
> > Why so? I'm open to any suggestions, choosing the right name for basically
> > anything is a gift I unfortunately do not possess, but truly desire to.
> >
>
> Hmm...  Looks like I got distracted whilst writing and didn't finish my
> though  I too have the gift of choosing names that cause angst for
> reviewers. Anyway - it's just a strange name for something that I think
> ends up being (what I call) the adapter or controller, e.g. in an .args
> file:
>
> -device vfio-pci,id=hostdev0,\
> sysfsdev=/sys/bus/mdev/devices/53764d0e-85a0-42b4-af5c-2046b460b1dc,bus=pci.0,\
> addr=0x3
>
> the "vfio-pci" is less a "deviceAPI" to me and more the "device
> mechanism or name" to handle the traffic. But I see that "device_api" is

You're right, but this way, we at least stay consistent with arguably poor
naming under sysfs - I know, might have been better off with letting the fantasy
off the leash, but hopefully this one's going to bite us in the back
less than usualyeah, right

> what the file name ends up being and that I assume was agreed upon by
> the consortium of those who have been arguing about the vGPU/mdev sysfs
> layout for far longer than I wanted to pay close attention, so we just
> get to deal with it.
>
> Now as for availableInstances - thankfully there's cut-n-paste to deal
> with that long name.  Ironically the name is far longer than the value
> as opposed to something like uuid/wwnn/wwpn where the name is far
> shorter than what the value turns out to be. Glad I don't have to type a
> uuid/wwnn/wwpn value too often and even happier I have cut-n-paste
>

Thanks for review, I'm going to push the series in a moment. I also created BZ
1452072 to track the feature and pasted the URL to patches 3-6.

Erik

>
> John

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

Re: [libvirt] [PATCH v4 4/6] nodedev: Introduce the mdev capability to a PCI parent device

2017-05-18 Thread John Ferlan 
[...]

>>> +static int
>>> +udevFillMdevType(struct udev_device *device,
>>> + const char *dir,
>>> + virNodeDevCapMdevTypePtr type)
>>> +{
>>> +int ret = -1;
>>> +char *attrpath = NULL;
>>> +
>>> +#define MDEV_GET_SYSFS_ATTR(attr_name, cb, ...)
>>>  \
>>> +do {   
>>>  \
>>> +if (virAsprintf(, "%s/%s", dir, #attr_name) < 0)  
>>>  \
>>> +goto cleanup;  
>>>  \
>>> +   
>>>  \
>>> +if (cb(device, attrpath, __VA_ARGS__) < 0) 
>>>  \
>>> +goto cleanup;  
>>>  \
>>> +   
>>>  \
>>> +VIR_FREE(attrpath);
>>>  \
>>> +} while (0)
>>>  \
>>> +
>>> +if (VIR_STRDUP(type->id, last_component(dir)) < 0)
>>> +goto cleanup;
>>> +
>>> +/* query udev for the attributes under subdirectories using the 
>>> relative
>>> + * path stored in @dir, i.e. 'mdev_supported_types/'
>>> + */
>>> +MDEV_GET_SYSFS_ATTR(name, udevGetStringSysfsAttr, >name);
>>
>> Does this imply that name isn't necessarily optional as defined in RNG?
>> Can '@name' not exist and if it is optional how does that adjust the macro?
> 
> There's no need for the macro to be changed - type->name == NULL in that case
> which means it won't be formatted to the XML, there's no issue in that the
> NULL's going to be handled gracefully all the way down from
> udevGetStringSysfsAttr.
> 

O
>>
>>> +MDEV_GET_SYSFS_ATTR(device_api, udevGetStringSysfsAttr, 
>>> >device_api);
>>> +MDEV_GET_SYSFS_ATTR(available_instances, udevGetUintSysfsAttr,
>>> +>available_instances, 10);
>>> +
>>> +#undef MDEV_GET_SYSFS_ATTR
>>> +
>>> +ret = 0;
>>> + cleanup:
>>> +VIR_FREE(attrpath);
>>> +return ret;
>>> +}
>>> +
>>
>>
>> With changes to
>>
>>  1. fix the Coverity issues
>>  2. determine whether the virNodeDeviceObjHasCap change is needed
>>  3. address the optional name
> ^ see my comment above
> 
>>
>> You have my:
>>
>> Reviewed-by: John Ferlan 
>>
>> While I'm not a fan of 'deviceAPI'
> 
> Why so? I'm open to any suggestions, choosing the right name for basically
> anything is a gift I unfortunately do not possess, but truly desire to.
> 

Hmm...  Looks like I got distracted whilst writing and didn't finish my
though  I too have the gift of choosing names that cause angst for
reviewers. Anyway - it's just a strange name for something that I think
ends up being (what I call) the adapter or controller, e.g. in an .args
file:

-device vfio-pci,id=hostdev0,\
sysfsdev=/sys/bus/mdev/devices/53764d0e-85a0-42b4-af5c-2046b460b1dc,bus=pci.0,\
addr=0x3

the "vfio-pci" is less a "deviceAPI" to me and more the "device
mechanism or name" to handle the traffic. But I see that "device_api" is
what the file name ends up being and that I assume was agreed upon by
the consortium of those who have been arguing about the vGPU/mdev sysfs
layout for far longer than I wanted to pay close attention, so we just
get to deal with it.

Now as for availableInstances - thankfully there's cut-n-paste to deal
with that long name.  Ironically the name is far longer than the value
as opposed to something like uuid/wwnn/wwpn where the name is far
shorter than what the value turns out to be. Glad I don't have to type a
uuid/wwnn/wwpn value too often and even happier I have cut-n-paste


John

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

Re: [libvirt] [PATCH v4 4/6] nodedev: Introduce the mdev capability to a PCI parent device

2017-05-18 Thread Erik Skultety 
On Wed, May 17, 2017 at 05:22:45PM -0400, John Ferlan wrote:
>
>
> On 05/15/2017 08:10 AM, Erik Skultety wrote:
> > The parent device needs to report the generic stuff about the supported
> > mediated devices types, like device API, available instances, type name,
> > etc. Therefore this patch introduces a new nested capability element of
> > type 'mdev_types' with the resulting XML of the following format:
> >
> > 
> >   ...
> >   
> > ...
> > 
> >   
> > optional_vendor_supplied_codename
> > vfio-pci
> > NUM
> >   
> > ...
> >   
> > ...
> >   
> > 
> >   
> >   ...
> > 
>
> I haven't followed all the discussions that closely - so a comment from
> the peanut gallery...  Since mdev_types is a sub-capability of
> type='pci', does the "vfio-pci" feel redundant? Ok just the -pci part...

Well, that would be mangling of the value we query from the sysfs and that
would again bring us to the point where people would rely on it. IMHO we should
expose the values verbatim as we find it under sysfs, do we actually do adjust
the attribute values after querying them from sysfs??
Anyhow, during one of the VFIO meetings we've also discussed a possibility of
someone coming up with vfio-pci v2 backend (though unlikely), in that aspect,
you really want to know what is the exact device API supported by that mdev
type (I have another unlikely and extremely 'visionary' scenario to support my
argument, but I'd rather keep it for myself :)).

>
> >
> > Signed-off-by: Erik Skultety 
> > ---
> >  docs/schemas/nodedev.rng   |  26 +
> >  src/conf/node_device_conf.c|  97 +
> >  src/conf/node_device_conf.h|  15 +++
> >  src/conf/virnodedeviceobj.c|   7 ++
> >  src/libvirt_private.syms   |   1 +
> >  src/node_device/node_device_udev.c | 119 
> > +
> >  .../pci__02_10_7_mdev_types.xml|  32 ++
> >  tests/nodedevxml2xmltest.c |   1 +
> >  8 files changed, 298 insertions(+)
> >  create mode 100644 tests/nodedevschemadata/pci__02_10_7_mdev_types.xml
> >
> > diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> > index 0f90a73c8..e0a2c5032 100644
> > --- a/docs/schemas/nodedev.rng
> > +++ b/docs/schemas/nodedev.rng
> > @@ -205,6 +205,32 @@
> >  
> >
> >  
> > +  
> > +
> > +  mdev_types
> > +
> > +
> > +  
> > +
> > +  
> > +
> > +
> > +  
> > +
> > +
> > +  
> > +vfio-pci
> > +  
> > +
> > +
> > +  
> > +
> > +  
> > +
> > +  
> > +   
> > +
> > +
> >
> >  
> >
> > diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> > index 40d71f277..f26b1ffc7 100644
> > --- a/src/conf/node_device_conf.c
> > +++ b/src/conf/node_device_conf.c
> > @@ -89,6 +89,19 @@ virNodeDevCapsDefParseString(const char *xpath,
> >
> >
> >  void
> > +virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type)
> > +{
> > +if (!type)
> > +return;
> > +
> > +VIR_FREE(type->id);
> > +VIR_FREE(type->name);
> > +VIR_FREE(type->device_api);
> > +VIR_FREE(type);
> > +}
> > +
> > +
> > +void
> >  virNodeDeviceDefFree(virNodeDeviceDefPtr def)
> >  {
> >  virNodeDevCapsDefPtr caps;
> > @@ -265,6 +278,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
> >  virBufferAsprintf(buf, "\n",
> >virPCIHeaderTypeToString(data->pci_dev.hdrType));
> >  }
> > +if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
> > +virBufferAddLit(buf, "\n");
> > +virBufferAdjustIndent(buf, 2);
> > +for (i = 0; i < data->pci_dev.nmdev_types; i++) {
> > +virNodeDevCapMdevTypePtr type = data->pci_dev.mdev_types[i];
> > +virBufferEscapeString(buf, "\n", type->id);
> > +virBufferAdjustIndent(buf, 2);
> > +if (type->name)
> > +virBufferEscapeString(buf, "%s\n",
> > +  type->name);
> > +virBufferEscapeString(buf, "%s\n",
> > +  type->device_api);
> > +virBufferAsprintf(buf,
> > +  
> > "%u\n",
> > +  type->available_instances);
> > +virBufferAdjustIndent(buf, -2);
> > +virBufferAddLit(buf, "\n");
> > +}
> > +virBufferAdjustIndent(buf, -2);
> > +virBufferAddLit(buf, "\n");
> > +}
> >  if (data->pci_dev.nIommuGroupDevices) {
> >  virBufferAsprintf(buf, "\n",
> >data->pci_dev.iommuGroupNumber);
> >

Re: [libvirt] [RFC] Fixing a regression caused by recent CPU driver changes

2017-05-18 Thread Pavel Hrdina 
On Thu, May 18, 2017 at 10:22:59AM +0200, Jiri Denemark wrote:
> Hi all,
> 
> when I was enhancing libvirt's guest CPU configuration code to be able
> to really ensure stable guest CPU ABI, I added a new attribute
> //cpu/@check which is nicely backward compatible... an old libvirt will
> just ignore it. However, even if check='full' will be ignored, an old
> libvirt will still see the updated CPU definition (features added or
> removed by the hypervisor will be shown there). And because we need QEMU
> 2.9.0 to check what features are going to be added or removed before we
> actually start the domain, migrating such domain to an older libvirt or
> QEMU may fail if QEMU enables a feature which is not supported by the
> host CPU. Known features causing problems are, e.g., x2apic, hypervisor,
> and arat. To make things even worse, updating a CPU definition with the
> automatically added/removed features can be done since QEMU 1.5.0.
> 
> Even save/restore or snapshot revert on a single host running new
> libvirt and QEMU < 2.9.0 is now affected by this regression.
> 
> The big question is how to fix the regression in a backward compatible
> way and still keep the ability to properly check guest CPU ABI with new
> enough libvirt and QEMU. Clearly, we need to keep both the original and
> the updated CPU definition (or one of them and a diff against the
> other).
> 
> I came up with the following options:
> 
> 1. When migrating a domain, the domain XML would contain the original
>CPU def while the updated one would be transferred in a migration
>cookie.
>- doesn't fix save/restore or snapshot revert
>  - could be fixed by not updating the CPU with QEMU < 2.9.0, but it
>won't work when restore is done on a different host with older
>QEMU (and yes, this happens in real life, e.g., with oVirt)
>- doesn't even fix migration after save/restore or snapshot revert

This is clearly not good enough.

> 2. Use migration cookie and change the save image format to contain
>additional data (something like migration cookie) which a new libvirt
>could make use of while old libvirt would ignore any additional data
>it doesn't know about
>- snapshot XML would need to be updated too, but that's trivial
>- cleanly changing save image format requires its version to be
>  increased and old libvirt will just refuse to read such image
>- this would fix save/restore on the same host or on a host with
>  older QEMU
>- doesn't fix restore on a different host running older libvirt
>  - even this is done by oVirt

This would be the best solution, however we might need to go with 
option 3.  The CPU updating is a new feature but it's enabled
automatically without any way how to disable it and in this case we
should not break anything.

Changing the save image format by introducing new feature to
seve/restore APIs is valid reason to increase a version so old libvirt
will fail if the new feature of save/restore is used.  Unfortunately
this is not the case, we need to update the save image format to make
different feature backward compatible which should not break the save
image compatibility.

> 3. Use migration cookie and change the save image format without
>increasing its version (and update snapshot XML)
>- this fixes all cases
>- technically possible by using one of the unused 32b ints and
>  adding \0 at the end of the domain XML followed by anothe XML with
>  the additional data (pointed to by the formerly unused uint32_t)
>- very ugly hack

It's not that ugly.  We just need to workaround a bad design used to
store domain XML in the save image.  Currently there is no easy and
nice way how to add additional data into the XML without breaking
backward compatibility and using \0 is the only backward compatible way
of doing it.  Luckily we have another 15 unused 32b ints :).

The ideal solution would be to use proper XML:


  
...
  
  
...
  
  
...
  
  ...


but since we store the domain XML directly as binary data the only way
how to store additional data for backward compatible features we need to
"extend" the current binary data with additional XML:


  ...

\0

  
...
  
  
...
  
  ...


and the newly used 32b int will just point to the beginning of the
additional data.

> 4. Format both CPUs in domain XML by adding a new subelement in 
>which would list all extra features disabled or enabled by QEMU
>- fixes all cases without the need to use migration cookie, change
>  save image format, or snapshot XML
>- but it may be very confusing for users and it would need to be
>  documented as "output only, don't touch staff"

I don't like it, this just puts an extra noise into the XML that is
irrelevant for users.

> 5. Add the additional data in 
>- a bad joke really
> 
> 
> So my preferred solution is 2. It breaks restore on a host with older
> libvirt, but it was never guaranteed to work, even

[libvirt] [PATCH 1/2] conf: add eim attribute to

2017-05-18 Thread Ján Tomko 
Add an attribute to control extended interrupt mode.

https://bugzilla.redhat.com/show_bug.cgi?id=1451282
---
 docs/formatdomain.html.in  | 10 +++
 docs/schemas/domaincommon.rng  |  5 
 src/conf/domain_conf.c | 20 ++
 src/conf/domain_conf.h |  1 +
 .../qemuxml2argv-intel-iommu-eim.xml   | 31 ++
 .../qemuxml2xmlout-intel-iommu-eim.xml |  1 +
 tests/qemuxml2xmltest.c|  1 +
 7 files changed, 69 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.xml
 create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-eim.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3135db4..b5026b1 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7436,6 +7436,16 @@ qemu-kvm -net nic,model=? /dev/null
   Since 3.4.0 (QEMU/KVM only)
 
   
+  eim
+  
+
+  The eim attribute with possible values
+  on and off can be used to
+  turn on extended interrupt mode. In combination with intremap
+  and split I/O APIC, this allows for more vCPUs to be used.
+  Since 3.4.0 (QEMU/KVM only)
+
+  
 
   
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f88e84a..144c281 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3959,6 +3959,11 @@
   
 
   
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9eba70a..2264d96 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14184,6 +14184,14 @@ virDomainIOMMUDefParseXML(xmlNodePtr node,
 }
 iommu->caching_mode = val;
 }
+VIR_FREE(tmp);
+if ((tmp = virXPathString("string(./driver/@eim)", ctxt))) {
+if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_XML_ERROR, _("unknown eim value: %s"), tmp);
+goto cleanup;
+}
+iommu->eim = val;
+}
 
 ret = iommu;
 iommu = NULL;
@@ -19856,6 +19864,14 @@ 
virDomainIOMMUDefCheckABIStability(virDomainIOMMUDefPtr src,
virTristateSwitchTypeToString(src->caching_mode));
 return false;
 }
+if (src->eim != dst->eim) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target domain IOMMU device eim value '%s' "
+ "does not match source '%s'"),
+   virTristateSwitchTypeToString(dst->eim),
+   virTristateSwitchTypeToString(src->eim));
+return false;
+}
 return true;
 }
 
@@ -24199,6 +24215,10 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
 virBufferAsprintf(, " caching_mode='%s'",
   
virTristateSwitchTypeToString(iommu->caching_mode));
 }
+if (iommu->eim != VIR_TRISTATE_SWITCH_ABSENT) {
+virBufferAsprintf(, " eim='%s'",
+  virTristateSwitchTypeToString(iommu->eim));
+}
 virBufferAddLit(, "/>\n");
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 34a3596..83e0672 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2211,6 +2211,7 @@ struct _virDomainIOMMUDef {
 virDomainIOMMUModel model;
 virTristateSwitch intremap;
 virTristateSwitch caching_mode;
+virTristateSwitch eim;
 };
 /*
  * Guest VM main configuration
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.xml
new file mode 100644
index 000..8642ed3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.xml
@@ -0,0 +1,31 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  288
+  
+hvm
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+
+  
+
+
+
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-eim.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-eim.xml
new file mode 12
index 000..9fbec36
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-eim.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index fed74d0..fff13e2 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1127,6 +1127,7 @@ mymain(void)
 QEMU_CAPS_MACHINE_IOMMU);
 DO_TEST("intel-iommu-ioapic", NONE);
 DO_TEST("intel-iommu-caching-mode", NONE);
+

[libvirt] [PATCH 0/2] Add eim attribute to iommu device

2017-05-18 Thread Ján Tomko 
Extended interrupt mode allows >255 vCPUs with q35-based machine types.

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

Ján Tomko (2):
  conf: add eim attribute to 
  qemu: format eim on intel-iommu command line

 docs/formatdomain.html.in  | 10 +++
 docs/schemas/domaincommon.rng  |  5 
 src/conf/domain_conf.c | 20 ++
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_capabilities.c   |  2 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 11 
 src/qemu/qemu_domain.c | 20 ++
 tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |  1 +
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |  1 +
 .../qemuxml2argv-intel-iommu-eim.args  | 19 +
 .../qemuxml2argv-intel-iommu-eim.xml   | 31 ++
 tests/qemuxml2argvtest.c   |  7 +
 .../qemuxml2xmlout-intel-iommu-eim.xml |  1 +
 tests/qemuxml2xmltest.c|  1 +
 15 files changed, 131 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.xml
 create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-eim.xml

-- 
2.10.2

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

[libvirt] [PATCH 2/2] qemu: format eim on intel-iommu command line

2017-05-18 Thread Ján Tomko 
This option turns on extended interrupt mode,
which allows more than 255 vCPUs.

https://bugzilla.redhat.com/show_bug.cgi?id=1451282
---
 src/qemu/qemu_capabilities.c |  2 ++
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c  | 11 +++
 src/qemu/qemu_domain.c   | 20 
 tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml |  1 +
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml |  1 +
 .../qemuxml2argv-intel-iommu-eim.args| 19 +++
 tests/qemuxml2argvtest.c |  7 +++
 8 files changed, 62 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-eim.args

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 546dfd7..7ea8505 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -371,6 +371,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "kernel-irqchip.split",
   "intel-iommu.intremap",
   "intel-iommu.caching-mode",
+  "intel-iommu.eim",
 );
 
 
@@ -1728,6 +1729,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsUSBNECXHCI[] = {
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = {
 { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP },
 { "caching-mode", QEMU_CAPS_INTEL_IOMMU_CACHING_MODE },
+{ "eim", QEMU_CAPS_INTEL_IOMMU_EIM },
 };
 
 /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index aa99fda..eba9814 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -409,6 +409,7 @@ typedef enum {
 QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT, /* -machine kernel_irqchip=split */
 QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */
 QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */
+QEMU_CAPS_INTEL_IOMMU_EIM, /* intel-iommu.eim */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4c1a266..f190d51 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6701,6 +6701,13 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
  "with this QEMU binary"));
 return -1;
 }
+if (iommu->eim != VIR_TRISTATE_SWITCH_ABSENT &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_EIM))  {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("iommu: eim is not supported "
+ "with this QEMU binary"));
+return -1;
+}
 break;
 case VIR_DOMAIN_IOMMU_MODEL_LAST:
 break;
@@ -6734,6 +6741,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd,
 virBufferAsprintf(, ",caching-mode=%s",
   
virTristateSwitchTypeToString(iommu->caching_mode));
 }
+if (iommu->eim != VIR_TRISTATE_SWITCH_ABSENT) {
+virBufferAsprintf(, ",eim=%s",
+  virTristateSwitchTypeToString(iommu->eim));
+}
 case VIR_DOMAIN_IOMMU_MODEL_LAST:
 break;
 }
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7c54f69..cd1825e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2663,6 +2663,9 @@ qemuDomainRecheckInternalPaths(virDomainDefPtr def,
 }
 
 
+#define QEMU_MAX_VCPUS_WITHOUT_EIM 255
+
+
 static int
 qemuDomainDefVcpusPostParse(virDomainDefPtr def)
 {
@@ -3071,6 +3074,23 @@ qemuDomainDefValidate(const virDomainDef *def,
 }
 }
 
+if (ARCH_IS_X86(def->os.arch) &&
+virDomainDefGetVcpusMax(def) > QEMU_MAX_VCPUS_WITHOUT_EIM) {
+if (!qemuDomainIsQ35(def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("more than %d vCPUs are only supported on "
+ "q35-based machine types"),
+   QEMU_MAX_VCPUS_WITHOUT_EIM);
+goto cleanup;
+}
+if (!def->iommu || def->iommu->eim != VIR_TRISTATE_SWITCH_ON) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("more than %d vCPUs require extended interrupt "
+ "mode enabled on the iommu device"),
+   QEMU_MAX_VCPUS_WITHOUT_EIM);
+}
+}
+
 if (qemuDomainDefValidateVideo(def) < 0)
 goto cleanup;
 
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
index e515678..01edbc8 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
@@ -206,6 +206,7 @@
   
   
   
+  
   2008000
   0
(v2.8.0)
diff --git

[libvirt] [PATCH 6/8] apparmor: include local apparmor profiles

2017-05-18 Thread Stefan Bader 
From: Felix Geyer 

Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
This allows the user to have a non-conffile that he can use to extend the
package delivered rules with extra content matching his special case.

This change adds the include directives to the apparmor profiles
for virt-aa-helper and libvirtd.

Additionally extended the build environment to carry template local
profiles and install them into the correct places. Without that the
include directives would prevent the profile from loading.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
Acked-by: Jamie Strandboge 
---
 examples/Makefile.am   | 14 ++
 examples/apparmor/local-usr.lib.libvirt.virt-aa-helper |  2 ++
 examples/apparmor/local-usr.sbin.libvirtd  |  2 ++
 examples/apparmor/usr.lib.libvirt.virt-aa-helper   |  3 +++
 examples/apparmor/usr.sbin.libvirtd|  3 +++
 5 files changed, 24 insertions(+)
 create mode 100644 examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
 create mode 100644 examples/apparmor/local-usr.sbin.libvirtd

diff --git a/examples/Makefile.am b/examples/Makefile.am
index 2956e14..16c7bf6 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -25,6 +25,8 @@ EXTRA_DIST = \
apparmor/libvirt-lxc \
apparmor/usr.lib.libvirt.virt-aa-helper \
apparmor/usr.sbin.libvirtd \
+   apparmor/local-usr.sbin.libvirtd \
+   apparmor/local-usr.lib.libvirt.virt-aa-helper \
lxcconvert/virt-lxc-convert \
polkit/libvirt-acl.rules \
$(wildcard $(srcdir)/systemtap/*.stp) \
@@ -74,6 +76,18 @@ apparmor_DATA = \
apparmor/usr.sbin.libvirtd \
$(NULL)
 
+localdir = $(apparmordir)/local
+local_DATA = \
+   apparmor/local-usr.sbin.libvirtd \
+   apparmor/local-usr.lib.libvirt.virt-aa-helper \
+   $(NULL)
+
+install-data-hook:
+   mv $(DESTDIR)$(localdir)/local-usr.sbin.libvirtd \
+  $(DESTDIR)$(localdir)/usr.sbin.libvirtd
+   mv $(DESTDIR)$(localdir)/local-usr.lib.libvirt.virt-aa-helper \
+  $(DESTDIR)$(localdir)/usr.lib.libvirt.virt-aa-helper
+
 abstractionsdir = $(apparmordir)/abstractions
 abstractions_DATA = \
apparmor/libvirt-qemu \
diff --git a/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
new file mode 100644
index 000..82c9c39
--- /dev/null
+++ b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
@@ -0,0 +1,2 @@
+# Site-specific additions and overrides for usr.lib.libvirt.virt-aa-helper.
+# For more details, please see /etc/apparmor.d/local/README.
diff --git a/examples/apparmor/local-usr.sbin.libvirtd 
b/examples/apparmor/local-usr.sbin.libvirtd
new file mode 100644
index 000..6e19f20
--- /dev/null
+++ b/examples/apparmor/local-usr.sbin.libvirtd
@@ -0,0 +1,2 @@
+# Site-specific additions and overrides for usr.sbin.libvirtd.
+# For more details, please see /etc/apparmor.d/local/README.
diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index 012080c..93ba74e 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -56,4 +56,7 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   /**.vmdk r,
   /**.[iI][sS][oO] r,
   /**/disk{,.*} r,
+
+  # Site-specific additions and overrides. See local/README for details.
+  #include 
 }
diff --git a/examples/apparmor/usr.sbin.libvirtd 
b/examples/apparmor/usr.sbin.libvirtd
index 353b039..c37d5ee 100644
--- a/examples/apparmor/usr.sbin.libvirtd
+++ b/examples/apparmor/usr.sbin.libvirtd
@@ -85,4 +85,7 @@
 
/usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix,
   }
+
+  # Site-specific additions and overrides. See local/README for details.
+  #include 
 }
-- 
2.7.4

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

[libvirt] [PATCH 7/8] appmor, libvirt-qemu: Add 9p support

2017-05-18 Thread Stefan Bader 
From: Serge Hallyn 

Add fowner and fsetid to libvirt-qemu profile.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 4 
 1 file changed, 4 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 89466c9..f04ce04 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -13,6 +13,10 @@
   capability setgid,
   capability setuid,
 
+  # for 9p
+  capability fsetid,
+  capability fowner,
+
   network inet stream,
   network inet6 stream,
 
-- 
2.7.4

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

[libvirt] [PATCH 1/8] virt-aa-helper: Ask for no deny rule for readonly disk elements

2017-05-18 Thread Stefan Bader 
From: Serge Hallyn 

Just because a disk element only requests read access doesn't mean
there may not be another readwrite request.

Using 'R' when creating the apparmor rule will prevent an implicit
write-deny rule to be created alongside. This does not mean write
is allowed but it would cause a denial message and probably more
relevant, allows to add write access later.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1554031

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 src/security/virt-aa-helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 5f5d1cd..d976a00 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -888,11 +888,11 @@ add_file_path(virDomainDiskDefPtr disk,
 
 if (depth == 0) {
 if (disk->src->readonly)
-ret = vah_add_file(buf, path, "r");
+ret = vah_add_file(buf, path, "R");
 else
 ret = vah_add_file(buf, path, "rw");
 } else {
-ret = vah_add_file(buf, path, "r");
+ret = vah_add_file(buf, path, "R");
 }
 
 if (ret != 0)
-- 
2.7.4

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

[libvirt] [PATCH 3/8] apparmor, virt-aa-helper: Allow aarch64 UEFI.

2017-05-18 Thread Stefan Bader 
From: William Grant 

Allow access to aarch64 UEFI images.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
Acked-by: Guido Günther 
---
 examples/apparmor/libvirt-qemu | 2 ++
 src/security/virt-aa-helper.c  | 4 +++-
 tests/virt-aa-helper-test  | 2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index e0988bb..89466c9 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -71,6 +71,8 @@
   /usr/share/seabios/** r,
   /usr/share/ovmf/** r,
   /usr/share/OVMF/** r,
+  /usr/share/AAVMF/** r,
+  /usr/share/qemu-efi/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index dd166c2..a2d5c21 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -513,7 +513,9 @@ valid_path(const char *path, const bool readonly)
 "/initrd",
 "/initrd.img",
 "/usr/share/OVMF/",  /* for OVMF images */
-"/usr/share/ovmf/"   /* for OVMF images */
+"/usr/share/ovmf/",  /* for OVMF images */
+"/usr/share/AAVMF/", /* for AAVMF images */
+"/usr/share/qemu-efi/"   /* for AAVMF images */
 };
 /* override the above with these */
 const char * const override[] = {
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 73f3080..51072f6 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -307,6 +307,8 @@ testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
 
 testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
 testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
+testfw "AAVMF" "/usr/share/AAVMF/AAVMF_CODE.fd"
+testfw "qemu-efi" "/usr/share/qemu-efi/QEMU_EFI.fd"
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
 touch "$tmpdir/initrd"
-- 
2.7.4

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

[libvirt] [PATCH 8/8] apparmor, libvirt-qemu: Add ppc64el related changes

2017-05-18 Thread Stefan Bader 
From: Serge Hallyn 

Updates profile to allow running on ppc64el.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1374554

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index f04ce04..2791dfc 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -77,6 +77,7 @@
   /usr/share/OVMF/** r,
   /usr/share/AAVMF/** r,
   /usr/share/qemu-efi/** r,
+  /usr/share/slof/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
@@ -102,6 +103,7 @@
   /usr/bin/qemu-system-or32 rmix,
   /usr/bin/qemu-system-ppc rmix,
   /usr/bin/qemu-system-ppc64 rmix,
+  /usr/bin/qemu-system-ppc64le rmix,
   /usr/bin/qemu-system-ppcemb rmix,
   /usr/bin/qemu-system-s390x rmix,
   /usr/bin/qemu-system-sh4 rmix,
@@ -158,3 +160,8 @@
   /etc/udev/udev.conf r,
   /sys/bus/ r,
   /sys/class/ r,
+
+  # for ppc device-tree access
+  @{PROC}/device-tree/ r,
+  @{PROC}/device-tree/** r,
+  /sys/firmware/devicetree/** r,
-- 
2.7.4

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

[libvirt] [PATCH 4/8] apparmor, virt-aa-helper: Allow access to libnl-3 config files

2017-05-18 Thread Stefan Bader 
From: Felix Geyer 

Allow access to libnl-3 config files

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
Acked-by: Guido Günther 
---
 examples/apparmor/usr.lib.libvirt.virt-aa-helper | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index 4a8f197..ee53c2c 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -16,6 +16,8 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   owner @{PROC}/[0-9]*/status r,
   @{PROC}/filesystems r,
 
+  /etc/libnl-3/classid r,
+
   # for hostdev
   /sys/devices/ r,
   /sys/devices/** r,
-- 
2.7.4

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

[libvirt] [PATCH 2/8] apparmor, virt-aa-helper: allow /usr/share/OVMF/ too

2017-05-18 Thread Stefan Bader 
From: Simon McVittie 

The split firmware and variables files introduced by
https://bugs.debian.org/764918 are in a different directory for some reason.
Let the virtual machine read both.

Extended by Christian Ehrhardt to generalize FW test (simplifies
additional testing on firmware files in future).

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
Acked-by: Guido Günther 
---
 examples/apparmor/libvirt-qemu |  1 +
 src/security/virt-aa-helper.c  |  1 +
 tests/virt-aa-helper-test  | 24 
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index a9020aa..e0988bb 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -70,6 +70,7 @@
   /usr/share/vgabios/** r,
   /usr/share/seabios/** r,
   /usr/share/ovmf/** r,
+  /usr/share/OVMF/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index d976a00..dd166c2 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly)
 "/vmlinuz",
 "/initrd",
 "/initrd.img",
+"/usr/share/OVMF/",  /* for OVMF images */
 "/usr/share/ovmf/"   /* for OVMF images */
 };
 /* override the above with these */
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 68e9399..73f3080 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -145,6 +145,20 @@ testme() {
 fi
 }
 
+testfw() {
+title="$1"
+fwpath="$2"
+
+if [ -f "$fwpath" ]; then
+sed -e "s,###UUID###,$uuid,g"  \
+-e "s,###DISK###,$disk1,g" \
+-e "s,,$fwpath,g" "$template_xml" > "$test_xml"
+testme "0" "$title" "-r -u $valid_uuid" "$test_xml"
+else
+echo "Skipping FW $title test. Could not find $fwpath"
+fi
+}
+
 # Expected failures
 echo "Expected failures:" >$output
 testme "1" "invalid arg" "-z"
@@ -291,14 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" 
-e "s,,$tm
 touch "$tmpdir/kernel"
 testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
 
-if [ -f /usr/share/ovmf/OVMF.fd ]; then
-sed -e "s,###UUID###,$uuid,g"  \
--e "s,###DISK###,$disk1,g" \
--e "s,,/usr/share/ovmf/OVMF.fd,g" "$template_xml" > 
"$test_xml"
-testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
-else
-echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd"
-fi
+testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
+testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
 touch "$tmpdir/initrd"
-- 
2.7.4

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

[libvirt] [PATCH 5/8] apparmor, virt-aa-helper: Explicit denies for host devices

2017-05-18 Thread Stefan Bader 
From: Felix Geyer 

Add explicit denies for disk devices to avoid cluttering dmesg with
(acceptable) denials (merged with a second patch which added more
disk device names).

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
Acked-by: Guido Günther 
---
 examples/apparmor/usr.lib.libvirt.virt-aa-helper | 9 +
 1 file changed, 9 insertions(+)

diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index ee53c2c..012080c 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -21,6 +21,15 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   # for hostdev
   /sys/devices/ r,
   /sys/devices/** r,
+  deny /dev/sd* r,
+  deny /dev/vd* r,
+  deny /dev/dm-* r,
+  deny /dev/drbd[0-9]* r,
+  deny /dev/dasd* r,
+  deny /dev/nvme* r,
+  deny /dev/zd[0-9]* r,
+  deny /dev/mapper/ r,
+  deny /dev/mapper/* r,
 
   /usr/{lib,lib64}/libvirt/virt-aa-helper mr,
   /{usr/,}sbin/apparmor_parser Ux,
-- 
2.7.4

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

[libvirt] Various apparmor related changes (part 1), version 2

2017-05-18 Thread Stefan Bader 
> Over the years there have been a bunch of changes to the
> apparmor profiles and/or virt-aa-helper which have been
> carried in Debian/Ubuntu but never made it upstream.
> 
> In an attempt to clean this up and generally improve the
> apparmor based environments, we (Christian and I) went
> over the changes, cleaned out cruft as much as possible 
> and would be sending out hunks of changes to this list
> for upstream inclusion.
> 
> I hope doing multiple but smaller rounds of submissions
> will make it simpler to get those reviewed and hopefully
> accepted.

For the second version I added acks, merged the patches
related to explicit device denials and local apparmor
profiles, and split the 9p support one (holding back the
part allowing link access for later or to be replaced by
a safer solution).
I also tried to improve the explanation in the description
of patch #1 (virt-aa-helper: Ask for no deny rule for readonly
disk elements).

Thanks,
Stefan

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

Re: [libvirt] [PATCH] pci: fix link maximum speed detection

2017-05-18 Thread Michal Privoznik 
On 05/16/2017 03:19 PM, Marek Marczykowski-Górecki wrote:
> Commit 8e09663 "pci: recognize/report GEN4 (PCIe 4.0) card 16GT/s Link
> speed" introduced another speed into enum, but mistakenly also altered
> field width, so one bit of link width was included there.
> 
> Signed-off-by: Marek Marczykowski-Górecki 
> ---
>  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 83c7e74..2c1b758 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -147,7 +147,7 @@ struct _virPCIDeviceList {
>  #define PCI_EXP_DEVCAP  0x4 /* Device capabilities */
>  #define PCI_EXP_DEVCAP_FLR (1<<28)  /* Function Level Reset */
>  #define PCI_EXP_LNKCAP  0xc /* Link Capabilities */
> -#define PCI_EXP_LNKCAP_SPEED0x0001f /* Maximum Link Speed */
> +#define PCI_EXP_LNKCAP_SPEED0xf /* Maximum Link Speed */
>  #define PCI_EXP_LNKCAP_WIDTH0x003f0 /* Maximum Link Width */
>  #define PCI_EXP_LNKSTA  0x12/* Link Status */
>  #define PCI_EXP_LNKSTA_SPEED0x000f  /* Negotiated Link Speed */
> 

ACKed and pushed.

Michal

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

[libvirt] [RFC] Fixing a regression caused by recent CPU driver changes

2017-05-18 Thread Jiri Denemark 
Hi all,

when I was enhancing libvirt's guest CPU configuration code to be able
to really ensure stable guest CPU ABI, I added a new attribute
//cpu/@check which is nicely backward compatible... an old libvirt will
just ignore it. However, even if check='full' will be ignored, an old
libvirt will still see the updated CPU definition (features added or
removed by the hypervisor will be shown there). And because we need QEMU
2.9.0 to check what features are going to be added or removed before we
actually start the domain, migrating such domain to an older libvirt or
QEMU may fail if QEMU enables a feature which is not supported by the
host CPU. Known features causing problems are, e.g., x2apic, hypervisor,
and arat. To make things even worse, updating a CPU definition with the
automatically added/removed features can be done since QEMU 1.5.0.

Even save/restore or snapshot revert on a single host running new
libvirt and QEMU < 2.9.0 is now affected by this regression.

The big question is how to fix the regression in a backward compatible
way and still keep the ability to properly check guest CPU ABI with new
enough libvirt and QEMU. Clearly, we need to keep both the original and
the updated CPU definition (or one of them and a diff against the
other).

I came up with the following options:

1. When migrating a domain, the domain XML would contain the original
   CPU def while the updated one would be transferred in a migration
   cookie.
   - doesn't fix save/restore or snapshot revert
 - could be fixed by not updating the CPU with QEMU < 2.9.0, but it
   won't work when restore is done on a different host with older
   QEMU (and yes, this happens in real life, e.g., with oVirt)
   - doesn't even fix migration after save/restore or snapshot revert

2. Use migration cookie and change the save image format to contain
   additional data (something like migration cookie) which a new libvirt
   could make use of while old libvirt would ignore any additional data
   it doesn't know about
   - snapshot XML would need to be updated too, but that's trivial
   - cleanly changing save image format requires its version to be
 increased and old libvirt will just refuse to read such image
   - this would fix save/restore on the same host or on a host with
 older QEMU
   - doesn't fix restore on a different host running older libvirt
 - even this is done by oVirt

3. Use migration cookie and change the save image format without
   increasing its version (and update snapshot XML)
   - this fixes all cases
   - technically possible by using one of the unused 32b ints and
 adding \0 at the end of the domain XML followed by anothe XML with
 the additional data (pointed to by the formerly unused uint32_t)
   - very ugly hack

4. Format both CPUs in domain XML by adding a new subelement in 
   which would list all extra features disabled or enabled by QEMU
   - fixes all cases without the need to use migration cookie, change
 save image format, or snapshot XML
   - but it may be very confusing for users and it would need to be
 documented as "output only, don't touch staff"

5. Add the additional data in 
   - a bad joke really


So my preferred solution is 2. It breaks restore on a host with older
libvirt, but it was never guaranteed to work, even though it usually
just worked. And we could just tell oVirt to fix there usage :-) Also
additional data in save image header may be very useful in the future; I
think I remember someone sighing about the inability to store more than
just domain XML in a saved image when they were trying to fix some
compatibility issues.

Any thoughts about the options or even completely new ideas are very
welcome.

Thanks,

Jirka

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

Re: [libvirt] [PATCH] virDomainDefCheckABIStabilityFlags: Check for memoryBacking

2017-05-18 Thread Peter Krempa 
On Thu, May 18, 2017 at 10:05:55 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1450349
> 
> Problem is, memoryBacking is part of guest ABI. Therefore
> changing it on migration/restore from an image can lead
> qemu/guest to rejecting the image.
> 
> At the same time, move other partial checks of virDomainMemtune
> into the same function: virDomainMemtuneCheckABIStability.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/domain_conf.c | 101 
> -
>  1 file changed, 75 insertions(+), 26 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9eba70a95..89b93f4ad 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19786,6 +19786,80 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr 
> src,
>  return virDomainDeviceInfoCheckABIStability(>info, >info);
>  }
>  
> +
> +static bool
> +virDomainMemtuneCheckABIStability(const virDomainDef *src,
> +  const virDomainDef *dst,
> +  unsigned int flags)
> +{
> +if (virDomainDefGetMemoryInitial(src) != 
> virDomainDefGetMemoryInitial(dst)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target domain max memory %lld "
> + "does not match source %lld"),
> +   virDomainDefGetMemoryInitial(dst),
> +   virDomainDefGetMemoryInitial(src));
> +return false;
> +}
> +
> +if (!(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE) &&
> +src->mem.cur_balloon != dst->mem.cur_balloon) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target domain current memory %lld "
> + "does not match source %lld"),
> +   dst->mem.cur_balloon,
> +   src->mem.cur_balloon);
> +return false;
> +}
> +
> +if (src->mem.max_memory != dst->mem.max_memory) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target maximum memory size '%llu' "
> + "doesn't match source '%llu'"),
> +   dst->mem.max_memory,
> +   src->mem.max_memory);
> +return false;
> +}
> +
> +if (src->mem.memory_slots != dst->mem.memory_slots) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target domain memory slots "
> + "count '%u' doesn't match source '%u'"),
> +   dst->mem.memory_slots,
> +   src->mem.memory_slots);
> +return false;
> +}
> +
> +if (src->mem.source != dst->mem.source) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target memoryBacking source '%s' doesn't "
> + "match source memoryBacking source'%s'"),
> +   virDomainMemorySourceTypeToString(dst->mem.source),
> +   virDomainMemorySourceTypeToString(src->mem.source));
> +return false;
> +}
AFAIK this is guest ABI only because qemu uses this to generate
hugepages rather than regular memory, not that it would be guest ABI
per se.

> +
> +if (src->mem.access != dst->mem.access) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target memoryBacking access '%s' doesn't "
> + "match access memoryBacking access'%s'"),
> +   virDomainMemoryAccessTypeToString(dst->mem.access),
> +   virDomainMemoryAccessTypeToString(src->mem.access));
> +return false;

Same here, shared access does not really transform to guest ABI.

> +}
> +
> +if (src->mem.allocation != dst->mem.allocation) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Target memoryBacking allocation '%s' doesn't "
> + "match allocation memoryBacking allocation'%s'"),
> +   
> virDomainMemoryAllocationTypeToString(dst->mem.allocation),
> +   
> virDomainMemoryAllocationTypeToString(src->mem.allocation));
> +return false;

This is definitely not guest ABI.


Note that there's a difference between guest ABI and differences in
configurations which are rejected by qemu.

I think we need a mechanism to deal with these in a different way
because it's possible that other hypervisors will have different ideas
on the compatibility of these options.


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

[libvirt] [PATCH] virDomainDefCheckABIStabilityFlags: Check for memoryBacking

2017-05-18 Thread Michal Privoznik 
https://bugzilla.redhat.com/show_bug.cgi?id=1450349

Problem is, memoryBacking is part of guest ABI. Therefore
changing it on migration/restore from an image can lead
qemu/guest to rejecting the image.

At the same time, move other partial checks of virDomainMemtune
into the same function: virDomainMemtuneCheckABIStability.

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c | 101 -
 1 file changed, 75 insertions(+), 26 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9eba70a95..89b93f4ad 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19786,6 +19786,80 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr 
src,
 return virDomainDeviceInfoCheckABIStability(>info, >info);
 }
 
+
+static bool
+virDomainMemtuneCheckABIStability(const virDomainDef *src,
+  const virDomainDef *dst,
+  unsigned int flags)
+{
+if (virDomainDefGetMemoryInitial(src) != 
virDomainDefGetMemoryInitial(dst)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target domain max memory %lld "
+ "does not match source %lld"),
+   virDomainDefGetMemoryInitial(dst),
+   virDomainDefGetMemoryInitial(src));
+return false;
+}
+
+if (!(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE) &&
+src->mem.cur_balloon != dst->mem.cur_balloon) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target domain current memory %lld "
+ "does not match source %lld"),
+   dst->mem.cur_balloon,
+   src->mem.cur_balloon);
+return false;
+}
+
+if (src->mem.max_memory != dst->mem.max_memory) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target maximum memory size '%llu' "
+ "doesn't match source '%llu'"),
+   dst->mem.max_memory,
+   src->mem.max_memory);
+return false;
+}
+
+if (src->mem.memory_slots != dst->mem.memory_slots) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target domain memory slots "
+ "count '%u' doesn't match source '%u'"),
+   dst->mem.memory_slots,
+   src->mem.memory_slots);
+return false;
+}
+
+if (src->mem.source != dst->mem.source) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target memoryBacking source '%s' doesn't "
+ "match source memoryBacking source'%s'"),
+   virDomainMemorySourceTypeToString(dst->mem.source),
+   virDomainMemorySourceTypeToString(src->mem.source));
+return false;
+}
+
+if (src->mem.access != dst->mem.access) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target memoryBacking access '%s' doesn't "
+ "match access memoryBacking access'%s'"),
+   virDomainMemoryAccessTypeToString(dst->mem.access),
+   virDomainMemoryAccessTypeToString(src->mem.access));
+return false;
+}
+
+if (src->mem.allocation != dst->mem.allocation) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target memoryBacking allocation '%s' doesn't "
+ "match allocation memoryBacking allocation'%s'"),
+   
virDomainMemoryAllocationTypeToString(dst->mem.allocation),
+   
virDomainMemoryAllocationTypeToString(src->mem.allocation));
+return false;
+}
+
+return true;
+}
+
+
 static bool
 virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src,
 virDomainMemoryDefPtr dst)
@@ -19940,37 +20014,12 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr 
src,
 goto error;
 }
 
-if (virDomainDefGetMemoryInitial(src) != 
virDomainDefGetMemoryInitial(dst)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain max memory %lld does not match source 
%lld"),
-   virDomainDefGetMemoryInitial(dst),
-   virDomainDefGetMemoryInitial(src));
+if (!virDomainMemtuneCheckABIStability(src, dst, flags))
 goto error;
-}
-if (!(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE) &&
-src->mem.cur_balloon != dst->mem.cur_balloon) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain current memory %lld does not match 
source %lld"),
-   dst->mem.cur_balloon, src->mem.cur_balloon);
-goto error;
-}
 
 if (!virDomainNumaCheckABIStability(src->numa, dst->numa))

[libvirt] [PATCH 2/2] virStream: Forbid negative seeks

2017-05-18 Thread Michal Privoznik 
Currently, we don't assign any meaning to that. Our current view
on virStream is that it's merely a pipe. And pipes don't support
seeking.

Signed-off-by: Michal Privoznik 
---
 src/internal.h   | 7 +++
 src/rpc/virnetclientstream.c | 1 +
 src/util/virfdstream.c   | 1 +
 3 files changed, 9 insertions(+)

diff --git a/src/internal.h b/src/internal.h
index 5a5a430a2..9e7ef553d 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -527,6 +527,13 @@
 goto label; \
 }   \
 } while (0)
+# define virCheckPositiveArgReturn(argname, retval) \
+do {\
+if (argname <= 0) { \
+virReportInvalidPositiveArg(argname);   \
+return retval;  \
+}   \
+} while (0)
 # define virCheckNonZeroArgGoto(argname, label) \
 do {\
 if (argname == 0) { \
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index 7b28d63db..a9bf271dc 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -367,6 +367,7 @@ virNetClientStreamSetHole(virNetClientStreamPtr st,
   unsigned int flags)
 {
 virCheckFlags(0, -1);
+virCheckPositiveArgReturn(length, -1);
 
 /* Shouldn't happen, But it's better to safe than sorry. */
 if (st->holeLength) {
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 6870d8846..7ee58be13 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -950,6 +950,7 @@ virFDStreamSendHole(virStreamPtr st,
 int ret = -1;
 
 virCheckFlags(0, -1);
+virCheckPositiveArgReturn(length, -1);
 
 virObjectLock(fdst);
 if (fdst->length) {
-- 
2.13.0

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

[libvirt] [PATCH 1/2] news: Document sparse streams

2017-05-18 Thread Michal Privoznik 
Signed-off-by: Michal Privoznik 
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 4cf14b0d5..52915ee2e 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,6 +35,16 @@
 
   
 
+  
+
+  Improved streams to efficiently transfer sparseness
+
+
+  New extension to virStream was implemented so that
+  virStorageVolDownload and virStorageVolUpload can preserve file
+  sparseness.
+
+  
 
 
   
-- 
2.13.0

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

[libvirt] [PATCH 0/2] Couple of sparse streams improvements

2017-05-18 Thread Michal Privoznik 
News entry & one simple fix.

Michal Privoznik (2):
  news: Document sparse streams
  virStream: Forbid negative seeks

 docs/news.xml| 10 ++
 src/internal.h   |  7 +++
 src/rpc/virnetclientstream.c |  1 +
 src/util/virfdstream.c   |  1 +
 4 files changed, 19 insertions(+)

-- 
2.13.0

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