date:20190329

Re: [libvirt] [PATCH 1/7] util: buffer: Remove struct member munging

2019-03-29 Thread Laine Stump


On 3/29/19 9:33 AM, Peter Krempa wrote:

This was meant to stop abusing the members directly, but we don't do
this for other internal structs. Additionally this did not stop the
test from touching the members. Remove the header obscurization.



I agree with you that this obfuscation does nothing in the face of a 
hostile (or ignorant) coder, but if we instead just make the real struct 
public then it won't be just ignorant devs who incorrectly use the 
internals of virBuffer. How about taking care of it with a 
virbufferpriv.h as we now do for several other structs whose internals 
we want to keep "private"? vircommandpriv.h is one good example.




Signed-off-by: Peter Krempa 
---
  src/util/virbuffer.c | 12 
  src/util/virbuffer.h | 16 ++--
  tests/virbuftest.c   |  2 +-
  3 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 12bdd13d39..8bb9c8e1fa 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -23,24 +23,12 @@
  #include 
  #include "c-ctype.h"

-#define __VIR_BUFFER_C__
-
  #include "virbuffer.h"
  #include "virerror.h"
  #include "virstring.h"

  #define VIR_FROM_THIS VIR_FROM_NONE

-/* If adding more fields, ensure to edit buf.h to match
-   the number of fields */
-struct _virBuffer {
-unsigned int size;
-unsigned int use;
-unsigned int error; /* errno value, or -1 for usage error */
-int indent;
-char *content;
-};
-
  /**
   * virBufferFail
   * @buf: the buffer
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index b399c90154..16cd8515d6 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -35,19 +35,15 @@
  typedef struct _virBuffer virBuffer;
  typedef virBuffer *virBufferPtr;

-# ifndef __VIR_BUFFER_C__
-#  define VIR_BUFFER_INITIALIZER { 0, 0, 0, 0, NULL }
+# define VIR_BUFFER_INITIALIZER { 0, 0, 0, 0, NULL }

-/* This struct must be kept in sync with the real struct
-   in the buf.c impl file */
  struct _virBuffer {
-unsigned int a;
-unsigned int b;
-unsigned int c;
-int d;
-char *e;
+unsigned int size;
+unsigned int use;
+unsigned int error; /* errno value, or -1 for usage error */
+int indent;
+char *content;
  };
-# endif

  const char *virBufferCurrentContent(virBufferPtr buf);
  char *virBufferContentAndReset(virBufferPtr buf);
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index 34f02b1281..b608da94d4 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -29,7 +29,7 @@ static int testBufInfiniteLoop(const void *data)
   * which was the case after the above addchar at the time of the bug.
   * This test is a bit fragile, since it relies on virBuffer internals.
   */
-if (virAsprintf(, "%*s", buf->a - buf->b - 1, "a") < 0)
+if (virAsprintf(, "%*s", buf->size - buf->use - 1, "a") < 0)
  goto out;

  if (info->doEscape)



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

[libvirt] [PATCH v2 0/4] apps: update the listed libvirt apps

2019-03-29 Thread Daniel P . Berrangé

Purge apps that are dead and fix broken links

Daniel P. Berrangé (4):
  apps: remove dead archipel project
  apps: update link for buildbot
  apps: drop link for zenoss software
  apps: remove VM Manager android app

 docs/apps.html.in | 39 +--
 1 file changed, 1 insertion(+), 38 deletions(-)

-- 
2.20.1

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

[libvirt] [PATCH v2 4/4] apps: remove VM Manager android app

2019-03-29 Thread Daniel P . Berrangé

The VM Manager app is no longer present on the Play store and while
Google shows a couple of hits they look like the typical untrustworthy
3rd party download redistributors rather than an official site.

Reviewed-by: Andrea Bolognani 
Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 33e205bd9e..209854b6ac 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -457,17 +457,6 @@
   
 
 
-Mobile applications
-
-
-  https://market.android.com/details?id=vm.manager;>VM 
Manager
-  
-VM Manager is VM (libvirt) manager (over SSH) application. VM Manager
-is an application for libvirt VM / Domain management over SSH.
-Please keep in mind that this software is under heavy development.
-  
-
-
 Other
 
 
-- 
2.20.1

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

[libvirt] [PATCH v2 2/4] apps: update link for buildbot

2019-03-29 Thread Daniel P . Berrangé

The libvirt specific page linked for buildbot is a 404. This replacement
link is the closest to what was originally linked.

Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index a6e6ee402b..5108994911 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -122,7 +122,7 @@
 Continuous Integration
 
 
-  https://buildbot.net/buildbot/docs/current/Libvirt.html;>BuildBot
+  http://docs.buildbot.net/latest/manual/configuration/workers-libvirt.html;>BuildBot
   
 BuildBot is a system to automate the compile/test cycle required
 by most software projects.  CVS commits trigger new builds, run on
-- 
2.20.1

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

[libvirt] [PATCH v2 3/4] apps: drop link for zenoss software

2019-03-29 Thread Daniel P . Berrangé

The page we link to is a 404 and github repo hasn't been touched since
2012 so is clearly dead.

Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 6 --
 1 file changed, 6 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 5108994911..33e205bd9e 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -353,12 +353,6 @@
 metrics. It supports pCPU, vCPU, memory, block device, network 
interface,
 and performance event metrics for each virtual guest.
   
-  https://community.zenoss.org/docs/DOC-4687;>Zenoss
-  
-The Zenoss libvirt Zenpack adds support for monitoring virtualization
-servers.  It has been tested with KVM, QEMU, VMware ESX, and VMware
-GSX.
-  
 
 
 Provisioning
-- 
2.20.1

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

[libvirt] [PATCH v2 1/4] apps: remove dead archipel project

2019-03-29 Thread Daniel P . Berrangé

The project website http://archipelproject.org/ is dead, reporting a
cloudflare error message

The git repo at https://github.com/ArchipelProject/Archipel/ hasn't
had a commit since Nov 2016, and the last release was a beta6 release
in 2013.

Reviewed-by: Andrea Bolognani 
Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 20 
 1 file changed, 20 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 9b67fd5399..a6e6ee402b 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -30,26 +30,6 @@
   
 
 
-Client/Server applications
-
-
-  http://archipelproject.org;>Archipel
-  
-Archipel is a libvirt-based solution to manage and supervise virtual
-machines.  It uses XMPP for all communication. There is no web
-service or custom protocol.  You just need at least one XMPP server,
-like eJabberd, to start playing with it.  This allows Archipel to
-work completely real time.  You never have to refresh the user
-interface, you'll be notified as soon as something happens. You can
-even use your favorite chat clients to command your infrastructure.
-  
-  
-Isn't it great to be able to open a chat conversation with your
-virtual machine and say things like "How are you today?" or "Hey,
-please reboot"?
-  
-
-
 Command line tools
 
 
-- 
2.20.1

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

Re: [libvirt] [PATCH v3 2/6] tests: add targets for building libvirt inside docker containers

2019-03-29 Thread Andrea Bolognani

On Wed, 2019-03-27 at 17:10 +, Daniel P. Berrangé wrote:
> The Travis CI system uses docker containers for its build environment.
> These are pre-built and hosted under quay.io/libvirt so that developers
> can use them for reproducing problems locally.

Throughout the commit message,

  s/docker/Docker/g

[...]
> To do a mingw build, it is currently possible to use the fedora-rawhide
> and request a different configure script

s/mingw/MinGW/
s/and/image and/

[...]
> In all cases the GIT source tree is cloned locally into a 'ci-tree/src'
> sub-directory which is then exposed to the container at '/src'. It is
> setup to facilitate VPATH build so the initial working directory
> is '/src/build'. An in source tree build can be requested instead
> by passing an empty string VPATH= arg to make.

The part about the initial working directory being the VPATH build
is no longer accurate. I also don't see a lot of value in spelling
out the paths, since they are mostly an implementation detail: I
would just write something like

  By default, a VPATH build will be performed; to request an in-tree
  build instead, pass VPATH= to make. In all cases, the git source
  tree is cloned locally to avoid unnecessary network access.

> The make rules are kept in a standalone file that is included into the
> main Makefile.am, so that it is possible to run them without having to
> invoke autotools first.
> 
> It is neccessary to disable the gnulib submodule commit check because
> this fails due to the way we have manually cloned submodule repos as
> primary git repos with their own .git directory, instead of letting
> git treat them as submodules in the top level .git directory.
> 
>   make[1]: Entering directory '/src/build'
>   fatal: Not a valid object name origin
>   fatal: run_command returned non-zero status for .gnulib
>   .
>   maint.mk: found non-public submodule commit
>   make: *** [/src/maint.mk:1448: public-submodule-commit] Error 1

This last part is interesting for people looking at the code but not
for users, so I'd leave it out of the commit message.

[...]
> +HERE = $(shell pwd)
> +
> +# Figure out name and path to this file. This isn't
> +# portable but we only care for modern GNU make
> +THIS_FILE = $(abspath $(lastword $(MAKEFILE_LIST)))
> +
> +# The directory holding content on the host that we will
> +# expose to the container.
> +SCRATCHDIR = $(HERE)/ci-tree

Since you don't use $(HERE) anywhere else, you could do

  SCRATCHDIR = $(shell pwd)/ci-tree

here.

[...]
> +# The directory holding the clone of the git repo that

Not sure what happened with that comment ;)

[...]
> +# The directory holding the source inside the
> +# container. ie where we told docker to expose

Again for all messages displayed to the user and comments,

  s/docker/Docker/g

> +# the $GIT/ci-tree directory from the host

This is actually $(HOST_SRCDIR).

[...]
> +# Relative directory to perform the build in. This
> +# defaults to using a separate build dir, but can be
> +# set to empty string for an in-source tree build.
> +CONT_VPATH = build

This should be called VPATH according to the commit message...

VPATH also seems like a better name to expose to users, so I'd say
change the code to follow the documentation rather than the other
way around.

[...]
> +# Avoid pulling submodules over the network by locally
> +# cloning them
> +SUBMODULES = .gnulib src/keycodemapdb

I still very much don't like having this hardcoded instead of
figured out at runtime by parsing the output of 'git submodule', but
we can take care of that in a follow-up patch.

[...]
> +prepare-tree:
> + @if test "$(REUSE)" != "1" ; then \
> + rm -rf ci-tree ; \
> + fi
> + @if ! test -d ci-tree ; then \
> + mkdir -p ci-tree/src; \
> + cp /etc/passwd ci-tree; \
> + cp /etc/group ci-tree; \

All instances of 'ci-tree' here, a few lines down, and also in
the ci-build@% and ci-shell@% targets, should be changed to
$(SCRATCHDIR).

[...]
> + for mod in $(SUBMODULES) ; \
> + do \
> + if test -d $(TOP)/$$mod ; \
> + then \
> + echo "Cloning $(TOP)/$$mod to 
> $(HOST_SRCDIR)/$$mod"; \
> + git clone $(GIT_ARGS) $(TOP)/$$mod 
> $(HOST_SRCDIR)/$$mod || exit 1; \
> + fi ; \
> + done ; \

You can rewrite this loop as

  for mod in $(SUBMODULES); \
  do \
test -d $(TOP)/$$mod || continue; \
echo ... \
git clone ... \
  done

to reduce indentation.

But I'm actually not sure what the check is there for: the list of
submodules *will be* correct, especially once we move away from
hardcoding it; even on a fresh clone with uninitialized submodules,
which is a situation we need to handle better, the check will not
help:

  $ make -f Makefile.ci ci-build@debian-9
  Checking if Docker is available and running...Cloning /home/test/libvirt to

Re: [libvirt] [PATCH 2/4] apps: update link for buildbot

2019-03-29 Thread Andrea Bolognani

On Fri, 2019-03-29 at 15:05 +, Daniel P. Berrangé wrote:
> On Fri, Mar 29, 2019 at 03:35:54PM +0100, Andrea Bolognani wrote:
> > I'm not sure what was on the page previously linked to...
> 
> No idea either.

I got curious and went Wayback (get it?), here's the most recent
copy I could find at the original URL:

  
https://web.archive.org/web/20110830190926/https://buildbot.net/buildbot/docs/current/Libvirt.html

which is consistent with the link being added a few months earlier
with

  commit ab3a43200c1fd5c39a7dc15ba0ec9d092c606240
  Author: Justin Clift 
  Date:   Thu Jan 20 12:52:21 2011 +1100

docs: added new entries to apps page, plus adjusted a few existing

Anyway, as you can see the contents have changed very little in the
intervening years, with the first three paragraphs at least being
virtually identical between versions.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/4] apps: update link for buildbot

2019-03-29 Thread Daniel P . Berrangé

On Fri, Mar 29, 2019 at 03:35:54PM +0100, Andrea Bolognani wrote:
> On Fri, 2019-03-29 at 14:12 +, Daniel P. Berrangé wrote:
> > The libvirt specific page linked for buildbot is a 404. There is no
> > stable replacement link, just ever changing version number specific
> > links per-release. Just link to the top of the project site instead.
> 
> Would either
> 
>   http://docs.buildbot.net/latest/manual/configuration/workers-libvirt.html
> 
> or
> 
>   http://docs.buildbot.net/current/manual/configuration/workers-libvirt.html
> 
> work?

I guess they would. I was looking for something like these but the lniks
i found were 404s

> I'm not sure what was on the page previously linked to...

No idea either.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 4/4] apps: remove VM Manager android app

2019-03-29 Thread Andrea Bolognani

On Fri, 2019-03-29 at 14:12 +, Daniel P. Berrangé wrote:
> The VM Manager app is no longer present on the Play store and while
> google shows a couple of hits they look like the typical untrustworthy

s/google/Google.

With that fixed,

  Reviewed-by: Andrea Bolognani 

Both this and 1/4 are safe for freeze.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 3/4] apps: update link for zenoss software

2019-03-29 Thread Andrea Bolognani

On Fri, 2019-03-29 at 14:12 +, Daniel P. Berrangé wrote:
> The page we link to is a 404 and there is not an obvious libvirt
> specific replacement, so use the project homepage instead.

I would say

  https://www.zenoss.com/product/zenpacks/libvirt-virtualization

looks like a reasonable replacement, but seeing as the most recent
release was in 2012 and the GitHub account hasn't been touched since
then either, I would probably drop the entry altogether.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/4] apps: update link for buildbot

2019-03-29 Thread Andrea Bolognani

On Fri, 2019-03-29 at 14:12 +, Daniel P. Berrangé wrote:
> The libvirt specific page linked for buildbot is a 404. There is no
> stable replacement link, just ever changing version number specific
> links per-release. Just link to the top of the project site instead.

Would either

  http://docs.buildbot.net/latest/manual/configuration/workers-libvirt.html

or

  http://docs.buildbot.net/current/manual/configuration/workers-libvirt.html

work?

I'm not sure what was on the page previously linked to...

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 6/6] qemu: caps: Remove pointless debug message in virQEMUCapsInitQMPMonitor

2019-03-29 Thread Erik Skultety

On Fri, Mar 29, 2019 at 01:26:34PM +0100, Peter Krempa wrote:
> Failure of qemuMonitorGetVersion is fatal now that we only support QMP
> based qemus. Remove the debug message since we report an error already.
>
> Signed-off-by: Peter Krempa 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 1/4] apps: remove dead archipel project

2019-03-29 Thread Andrea Bolognani

On Fri, 2019-03-29 at 14:12 +, Daniel P. Berrangé wrote:
> The project website http://archipelproject.org/ is dead, reporting a
> cloudflare error message
> 
> The git repo at https://github.com/ArchipelProject/Archipel/ hasn't
> had a commit since Nov 2016, and the last release was a beta6 release
> in 2013.

The last commit message is literally 'use python2'. Yeesh.

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 5/6] qemu: caps: Remove cleanup section in virQEMUCapsInitQMPMonitor

2019-03-29 Thread Erik Skultety

On Fri, Mar 29, 2019 at 01:26:33PM +0100, Peter Krempa wrote:
> Signed-off-by: Peter Krempa 
> ---

Let's include the TCG equivalent of the same function.

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 4/6] qemu: caps: Don't leak package name string in virQEMUCapsInitQMPMonitor

2019-03-29 Thread Erik Skultety

On Fri, Mar 29, 2019 at 01:26:32PM +0100, Peter Krempa wrote:
> If the detected qemu version is below our required version 'package'
> would be leaked.
>
> Signed-off-by: Peter Krempa 
> ---
Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH] news: Update for 5.2.0 release

2019-03-29 Thread Andrea Bolognani

Signed-off-by: Andrea Bolognani 
---
 docs/news.xml | 37 +
 1 file changed, 37 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2067830848..fcf8520132 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -148,8 +148,45 @@
   tables are not required.
 
   
+  
+
+  Don't default to building the QEMU driver
+
+
+  Historically, the QEMU driver has been special in that it was
+  enabled by default, with the option to explicitly opt-out of it;
+  starting now, we're enabling it opportunistically if we detect that
+  all requirements are available, just like we do with other drivers.
+
+  
 
 
+  
+
+  virt-host-validate: Fix IOMMU check on s390x
+
+  
+  
+
+  qemu: Allow creating pSeries guests with graphics and no USB mouse
+
+
+  It's now possible to prevent libvirt from automatically adding a
+  USB mouse to pSeries guests by including a USB tablet in the input
+  XML: doing so is desiderable as using a tablet results in a much
+  better user experience when working with GUIs.
+
+  
+  
+
+  qemu: Set $HOME and XGD variables for qemu:///system guests
+
+
+  This avoids files being accidentally created under / or
+  the guests not being able to start because they lack the necessary
+  permissions to write to that location.
+
+  
 
   
   
-- 
2.20.1

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

[libvirt] [PATCH 0/4] apps: update the listed libvirt apps

2019-03-29 Thread Daniel P . Berrangé

Purge apps that are dead and fix broken links

Daniel P. Berrangé (4):
  apps: remove dead archipel project
  apps: update link for buildbot
  apps: update link for zenoss software
  apps: remove VM Manager android app

 docs/apps.html.in | 35 ++-
 1 file changed, 2 insertions(+), 33 deletions(-)

-- 
2.20.1

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

[libvirt] [PATCH 2/4] apps: update link for buildbot

2019-03-29 Thread Daniel P . Berrangé

The libvirt specific page linked for buildbot is a 404. There is no
stable replacement link, just ever changing version number specific
links per-release. Just link to the top of the project site instead.

Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index a6e6ee402b..ed12f06d73 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -122,7 +122,7 @@
 Continuous Integration
 
 
-  https://buildbot.net/buildbot/docs/current/Libvirt.html;>BuildBot
+  https://buildbot.net/;>BuildBot
   
 BuildBot is a system to automate the compile/test cycle required
 by most software projects.  CVS commits trigger new builds, run on
-- 
2.20.1

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

[libvirt] [PATCH 3/4] apps: update link for zenoss software

2019-03-29 Thread Daniel P . Berrangé

The page we link to is a 404 and there is not an obvious libvirt
specific replacement, so use the project homepage instead.

Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index ed12f06d73..fec66771be 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -353,7 +353,7 @@
 metrics. It supports pCPU, vCPU, memory, block device, network 
interface,
 and performance event metrics for each virtual guest.
   
-  https://community.zenoss.org/docs/DOC-4687;>Zenoss
+  https://community.zenoss.com;>Zenoss
   
 The Zenoss libvirt Zenpack adds support for monitoring virtualization
 servers.  It has been tested with KVM, QEMU, VMware ESX, and VMware
-- 
2.20.1

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

[libvirt] [PATCH 1/4] apps: remove dead archipel project

2019-03-29 Thread Daniel P . Berrangé

The project website http://archipelproject.org/ is dead, reporting a
cloudflare error message

The git repo at https://github.com/ArchipelProject/Archipel/ hasn't
had a commit since Nov 2016, and the last release was a beta6 release
in 2013.

Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 20 
 1 file changed, 20 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 9b67fd5399..a6e6ee402b 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -30,26 +30,6 @@
   
 
 
-Client/Server applications
-
-
-  http://archipelproject.org;>Archipel
-  
-Archipel is a libvirt-based solution to manage and supervise virtual
-machines.  It uses XMPP for all communication. There is no web
-service or custom protocol.  You just need at least one XMPP server,
-like eJabberd, to start playing with it.  This allows Archipel to
-work completely real time.  You never have to refresh the user
-interface, you'll be notified as soon as something happens. You can
-even use your favorite chat clients to command your infrastructure.
-  
-  
-Isn't it great to be able to open a chat conversation with your
-virtual machine and say things like "How are you today?" or "Hey,
-please reboot"?
-  
-
-
 Command line tools
 
 
-- 
2.20.1

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

[libvirt] [PATCH 4/4] apps: remove VM Manager android app

2019-03-29 Thread Daniel P . Berrangé

The VM Manager app is no longer present on the Play store and while
google shows a couple of hits they look like the typical untrustworthy
3rd party download redistributors rather than an official site.

Signed-off-by: Daniel P. Berrangé 
---
 docs/apps.html.in | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index fec66771be..79d50725f1 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -463,17 +463,6 @@
   
 
 
-Mobile applications
-
-
-  https://market.android.com/details?id=vm.manager;>VM 
Manager
-  
-VM Manager is VM (libvirt) manager (over SSH) application. VM Manager
-is an application for libvirt VM / Domain management over SSH.
-Please keep in mind that this software is under heavy development.
-  
-
-
 Other
 
 
-- 
2.20.1

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

Re: [libvirt] [PATCH 3/6] qemu: capabilities: Move logic deciding whether to probe into probing functions

2019-03-29 Thread Erik Skultety

On Fri, Mar 29, 2019 at 01:26:31PM +0100, Peter Krempa wrote:
> Most probing functions in virQEMUCapsInitQMPMonitor decide internally if
> there's anything to do and return success if there isn't. Move the
> decision logic for virQEMUCapsProbeQMPGICCapabilities,
> virQEMUCapsProbeQMPSEVCapabilities, and virQEMUCapsProbeQMPSchemaCapabilities
> into the function itself so that virQEMUCapsInitQMPMonitor looks tidy.
>
> Signed-off-by: Peter Krempa 
> ---

You might want to split the adjustments into 3 patches, your call.

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v2 1/2] qemu: Fix "boolen" typo in API doc

2019-03-29 Thread Christophe Fergeau

Hey,

On Wed, Mar 27, 2019 at 01:32:44PM +0100, Ján Tomko wrote:
> Please use --cover-letter for series with two or more patches for neater
> alignment of e-mails.
> 
> On Wed, Mar 27, 2019 at 12:21:24PM +0100, Christophe Fergeau wrote:
> > This also adjusts the argument name which should be 'isListen' in both
> > cases rather than 'listen'.
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> > Changes since v1:
> > - really fix 'boolen' typo
> > 
> > src/qemu/qemu_command.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index f81d20e5f7..82675c5d4e 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -876,7 +876,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
> > 
> > /* qemuBuildTLSx509BackendProps:
> >  * @tlspath: path to the TLS credentials
> > - * @listen: boolen listen for client or server setting
> > + * @isListen: boolean listen for client or server setting
> 
> Both the rename and the typo fix could be qualified as trivial.
> However the description does not make much sense to me.

To me neither. Maybe this?
* @isListen: boolean indicating if this is a client or server TLS credentials

> 
> >  * @verifypeer: boolean to enable peer verification (form of authorization)
> 
> peer verification is also used for authentication, as of
> commit 441d3eb6d1be940a67ce45a286602a967601b157 (tag: CVE-2017-1000256)

Just need to append /authentication to the end?
* @verifypeer: boolean to enable peer verification (form of 
authorization/authentication)


Christophe


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

Re: [libvirt] [PATCH 2/6] qemu: caps: Aggregate all caps post-processing into a function

2019-03-29 Thread Erik Skultety

On Fri, Mar 29, 2019 at 01:26:30PM +0100, Peter Krempa wrote:
> Some caps are cleared according to some more advanced logic after
> detection. Split all that logic out into virQEMUCapsInitProcessCaps.
>
> Signed-off-by: Peter Krempa 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [Qemu-devel] [PULL 04/14] audio: -audiodev command line option basic implementation

2019-03-29 Thread Markus Armbruster

Pavel Hrdina  writes:

> On Fri, Mar 29, 2019 at 11:12:55AM +0100, Markus Armbruster wrote:
>> Pavel Hrdina  writes:
>> 
>> > On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
>> >> Eric Blake  writes:
>> >> 
>> >> > On 3/28/19 3:06 PM, Eric Blake wrote:
>> >> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
>> >> >>> Markus Armbruster  writes:
>> >>  Pavel Hrdina  writes:
>> >> >> 
>> >> > I'm glad that this is merged now and I wanted to start working on
>> >> > libvirt patches, but there is one big issue with this command,
>> >> > it's not introspectable by query-command-line-options.
>> >> [...]
>> >> > Adding Markus to CC so we can figure out how to wire up the
>> >> > introspection for such command line options.
>> >> [...]
>> >> >>> Command line options are actually defined in qemu-options.hx, which 
>> >> >>> gets
>> >> >>> massaged into qemu_options[].  For each option, qemu_options[] gives 
>> >> >>> us
>> >> >>> the option name (without leading '-'), and whether the option takes an
>> >> >>> argument.
>> >> >>>
>> >> >>> This is less information per option than q-c-l-o provides now.  Can we
>> >> >>> add it to its output anyway without confusing existing users?
>> >> [...]
>> >> >>> Alternatives:
>> >> [...]
>> >> >>> 5. Screw it, create a new query command to return just the information
>> >> >>>from qemu_options[].
>> >
>> > Hi Markus,
>> >
>> > Thanks for looking into this issue, it would be perfect to solve it
>> > before QEMU 4.0 is released.
>> 
>> To be honest, I wouldn't bet my own money on 4.0 at this point.
>> 
>> I understand why you're eager to switch libvirt to -audiodev, it's such
>> a massive improvement over the traditional mess.  But is it urgent?
>> Does it fix bugs?  Does it add features?  Sure it can't wait for 4.1?
>
> It's definitely not urgent, the audio situation is broken the whole
> time so we can wait for 4.1.  It will help us to fix some bugs but
> nothing critical.

Let's aim for something decent in 4.1.  Perhaps alternative 5.  Perhaps
even something that can grow into real command line introspection.
Perhaps both.

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

[libvirt] [PATCH 5/5] qemu: domain: Forbid copy_on_read option also for floppies

2019-03-29 Thread Peter Krempa

Using copy_on_read for removable disks is a hassle. It also does not
work for CDROMs at all as the image is supposed to be read-only and we
might ignore it for floppies when they are started as empty. Forbid it
for floppies completely rather than trying to support what probably
nobody is using.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e903d36fec..2480211194 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5041,11 +5041,21 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef 
*disk,
 return -1;
 }

-if (disk->src->readonly && disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("copy_on_read is not compatible with read-only disk 
'%s'"),
-   disk->dst);
-return -1;
+if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) {
+if (disk->src->readonly) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("copy_on_read is not compatible with read-only 
disk '%s'"),
+   disk->dst);
+return -1;
+}
+
+if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("copy_on_read is not supported with removable 
disk '%s'"),
+   disk->dst);
+return -1;
+}
 }

 if (disk->geometry.cylinders > 0 &&
-- 
2.20.1

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

[libvirt] [PATCH 4/5] qemu: hotplug: Disallow media change while blockjob is active

2019-03-29 Thread Peter Krempa

Until the block job completes we can't change the disk chain. Removal
would fail as the block job still has reference to the chain.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 150da34b4a..34249bd030 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -852,10 +852,17 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virStorageSourcePtr oldsrc = disk->src;
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 bool sharedAdded = false;
 int ret = -1;
 int rc;

+if (diskPriv->blockjob && qemuBlockJobIsRunning(diskPriv->blockjob)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("can't change media while a block job is running on 
the device"));
+return -1;
+}
+
 disk->src = newsrc;

 if (virDomainDiskTranslateSourcePool(disk) < 0)
-- 
2.20.1

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

[libvirt] [PATCH 1/5] qemu: domain: Use VIR_AUTOFREE in qemuDomainObjPrivateXMLParseBlockjobs

2019-03-29 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 54afb6dd7b..e903d36fec 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2668,14 +2668,13 @@ static int
 qemuDomainObjPrivateXMLParseBlockjobs(qemuDomainObjPrivatePtr priv,
   xmlXPathContextPtr ctxt)
 {
-char *active;
+VIR_AUTOFREE(char *) active = NULL;
 int tmp;

 if ((active = virXPathString("string(./blockjobs/@active)", ctxt)) &&
 (tmp = virTristateBoolTypeFromString(active)) > 0)
 priv->reconnectBlockjobs = tmp;

-VIR_FREE(active);
 return 0;
 }

-- 
2.20.1

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

[libvirt] [PATCH 2/5] qemu: hotplug: Remove unused copies of virQEMUDriverConfigPtr

2019-03-29 Thread Peter Krempa

qemuDomainChangeGraphicsPasswords and qemuDomainRemoveHostDevice
don't use 'cfg' any more.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4e94d80f21..0318547bd8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4212,7 +4212,6 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
 const char *connected = NULL;
 const char *password;
 int ret = -1;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

 if (!auth->passwd && !defaultPasswd) {
 ret = 0;
@@ -4248,7 +4247,6 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
 ret = -1;
  cleanup:
 VIR_FREE(validTo);
-virObjectUnref(cfg);
 return ret;
 }

@@ -4657,7 +4655,6 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 virDomainNetDefPtr net = NULL;
 size_t i;
 int ret = -1;
@@ -4768,7 +4765,6 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
  cleanup:
 VIR_FREE(drivealias);
 VIR_FREE(objAlias);
-virObjectUnref(cfg);
 return ret;
 }

-- 
2.20.1

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

[libvirt] [PATCH 3/5] qemu: hotplug: Use VIR_AUTOUNREF for virQEMUDriverConfigPtr

2019-03-29 Thread Peter Krempa

Unref the config pointer automatically in code paths which get a local
copy.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0318547bd8..150da34b4a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -151,7 +151,7 @@ qemuHotplugPrepareDiskSourceAccess(virQEMUDriverPtr driver,
virStorageSourcePtr src,
bool teardown)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 const char *srcstr = NULLSTR(src->path);
 int ret = -1;
 virErrorPtr orig_err = NULL;
@@ -195,7 +195,6 @@ qemuHotplugPrepareDiskSourceAccess(virQEMUDriverPtr driver,

  cleanup:
 virErrorRestore(_err);
-virObjectUnref(cfg);

 return ret;
 }
@@ -850,7 +849,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virStorageSourcePtr newsrc,
bool force)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virStorageSourcePtr oldsrc = disk->src;
 bool sharedAdded = false;
@@ -917,7 +916,6 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 if (oldsrc)
 disk->src = oldsrc;

-virObjectUnref(cfg);
 return ret;
 }

@@ -936,7 +934,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuHotplugDiskSourceDataPtr diskdata = NULL;
 char *devstr = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);

 if (qemuHotplugPrepareDiskSourceAccess(driver, vm, disk->src, false) < 0)
 goto cleanup;
@@ -987,7 +985,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
 qemuHotplugDiskSourceDataFree(diskdata);
 qemuDomainSecretDiskDestroy(disk);
 VIR_FREE(devstr);
-virObjectUnref(cfg);
 return ret;

  exit_monitor:
@@ -1367,7 +1364,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 bool iface_connected = false;
 virDomainNetType actualType;
 virNetDevBandwidthPtr actualBandwidth;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 virDomainCCWAddressSetPtr ccwaddrs = NULL;
 size_t i;
 char *charDevAlias = NULL;
@@ -1707,7 +1704,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 VIR_FREE(vhostfd);
 VIR_FREE(vhostfdName);
 VIR_FREE(charDevAlias);
-virObjectUnref(cfg);
 virDomainCCWAddressSetFree(ccwaddrs);

 return ret;
@@ -1752,7 +1748,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
 bool teardownlabel = false;
 bool teardowndevice = false;
 int backend;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 unsigned int flags = 0;

 if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
@@ -1868,7 +1864,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
 VIR_FREE(devstr);
 VIR_FREE(configfd_name);
 VIR_FORCE_CLOSE(configfd);
-virObjectUnref(cfg);

 return 0;

@@ -1892,7 +1887,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
 VIR_FORCE_CLOSE(configfd);

  cleanup:
-virObjectUnref(cfg);
 return -1;
 }

@@ -2005,7 +1999,7 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver,
const char **secAlias)
 {
 int ret = -1;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainChrSourcePrivatePtr chrSourcePriv;
 qemuDomainSecretInfoPtr secinfo = NULL;
@@ -2050,7 +2044,6 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver,
  cleanup:
 virJSONValueFree(tlsProps);
 virJSONValueFree(secProps);
-virObjectUnref(cfg);

 return ret;
 }
@@ -2063,7 +2056,7 @@ qemuDomainDelChardevTLSObjects(virQEMUDriverPtr driver,
const char *inAlias)
 {
 int ret = -1;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 char *tlsAlias = NULL;
 char *secAlias = NULL;
@@ -2099,7 +2092,6 @@ qemuDomainDelChardevTLSObjects(virQEMUDriverPtr driver,
  cleanup:
 VIR_FREE(tlsAlias);
 VIR_FREE(secAlias);
-

[libvirt] [PATCH 0/5] qemu: hotplug: Media change improvements (blockdev-add saga)

2019-03-29 Thread Peter Krempa

First patch is not entirely relevant in this series.

Peter Krempa (5):
  qemu: domain: Use VIR_AUTOFREE in
qemuDomainObjPrivateXMLParseBlockjobs
  qemu: hotplug: Remove unused copies of virQEMUDriverConfigPtr
  qemu: hotplug: Use VIR_AUTOUNREF for virQEMUDriverConfigPtr
  qemu: hotplug: Disallow media change while blockjob is active
  qemu: domain: Forbid copy_on_read option also for floppies

 src/qemu/qemu_domain.c  | 23 +--
 src/qemu/qemu_hotplug.c | 51 -
 2 files changed, 36 insertions(+), 38 deletions(-)

-- 
2.20.1

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

Re: [libvirt] [PATCH 1/6] qemu: caps: Separate capabilities based on qemu version

2019-03-29 Thread Erik Skultety

On Fri, Mar 29, 2019 at 01:26:29PM +0100, Peter Krempa wrote:
> virQEMUCapsInitQMPMonitor is massive now since it collects calls to the
> various probing functions and also version based capabilities. Split
> out the version based caps into a separate function.
>
> Signed-off-by: Peter Krempa 
> ---
Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH 3/7] util: json: Use virBuffer in JSON->string conversion

2019-03-29 Thread Peter Krempa

The last step of the conversion involves copying of the generated JSON
into a separate string. We can use a virBuffer to do this as this will
also allow to subsequently use the buffer when we actually need to do
some other formatting of the string.

Signed-off-by: Peter Krempa 
---
 src/util/virjson.c | 38 +-
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index d5d66f879f..7dfc589944 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -28,6 +28,7 @@
 #include "virlog.h"
 #include "virstring.h"
 #include "virutil.h"
+#include "virbuffer.h"

 #if WITH_YAJL
 # include 
@@ -1969,17 +1970,18 @@ virJSONValueToStringOne(virJSONValuePtr object,
 }


-char *
-virJSONValueToString(virJSONValuePtr object,
+static int
+virJSONValueToBuffer(virJSONValuePtr object,
+ virBufferPtr buf,
  bool pretty)
 {
 yajl_gen g;
 const unsigned char *str;
-char *ret = NULL;
 yajl_size_t len;
 # ifndef WITH_YAJL2
 yajl_gen_config conf = { pretty ? 1 : 0, pretty ? "  " : " "};
 # endif
+int ret = -1;

 VIR_DEBUG("object=%p", object);

@@ -2009,13 +2011,12 @@ virJSONValueToString(virJSONValuePtr object,
 goto cleanup;
 }

-ignore_value(VIR_STRDUP(ret, (const char *)str));
+virBufferAdd(buf, (const char *) str, len);
+ret = 0;

  cleanup:
 yajl_gen_free(g);

-VIR_DEBUG("result=%s", NULLSTR(ret));
-
 return ret;
 }

@@ -2030,17 +2031,36 @@ virJSONValueFromString(const char *jsonstring 
ATTRIBUTE_UNUSED)
 }


-char *
-virJSONValueToString(virJSONValuePtr object ATTRIBUTE_UNUSED,
+static int
+virJSONValueToBuffer(virJSONValuePtr object ATTRIBUTE_UNUSED,
+ virBufferPtr buf ATTRIBUTE_UNUSED,
  bool pretty ATTRIBUTE_UNUSED)
 {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No JSON parser implementation is available"));
-return NULL;
+return -1;
 }
 #endif


+char *
+virJSONValueToString(virJSONValuePtr object,
+ bool pretty)
+{
+VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+char *ret = NULL;
+
+if (virJSONValueToBuffer(object, , pretty) < 0)
+return NULL;
+
+ret = virBufferContentAndReset();
+
+VIR_DEBUG("result=%s", NULLSTR(ret));
+
+return ret;
+}
+
+
 /**
  * virJSONStringReformat:
  * @jsonstr: string to reformat
-- 
2.20.1

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

[libvirt] [PATCH 6/7] qemu: monitor: Remove few debug statements

2019-03-29 Thread Peter Krempa

The internal qemu machinery already logs the sent message via the PROBE
point in qemuMonitorSend and the monitor receive function. Those are way
better as they are easy grepable. Remove the additional ones from the
monitor code which just duplicate the sent data.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_monitor_json.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 743a88b914..c7a7e3fa56 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -301,14 +301,8 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,
 msg.txLength = strlen(msg.txBuffer);
 msg.txFD = scm_fd;

-VIR_DEBUG("Send command '%s' for write with FD %d", cmdstr, scm_fd);
-
 ret = qemuMonitorSend(mon, );

-VIR_DEBUG("Receive command reply ret=%d rxObject=%p",
-  ret, msg.rxObject);
-
-
 if (ret == 0) {
 if (!msg.rxObject) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-- 
2.20.1

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

[libvirt] vcpupin reports bogus vcpu affinities

2019-03-29 Thread Allen, John

Sent this out to the list a few days ago, but never saw it appear on the
archives. Resending--hopefully this makes it to the list.

---

For pinned vcpus, vcpupin will report inaccurate affinity values on machines
with high core counts (256 cores in my case). The problem is produced as
follows:

$ virsh vcpupin myguest 0 4

$ virsh vcpupin myguest 0

 VCPU   CPU Affinity
---
 0  4,192,194,196-197

Running taskset on the qemu threads shows the correct affinity, so this seems
to be a reporting problem. Strangely, the value "192" is significant. If I pin
a cpu greater than 192, the problem no longer appears.

I believe the cause of the problem in my case is that in this case in
src/conf/domain_conf.c:virDomainDefGetVcpuPinInfoHelper:

...
if (vcpu && vcpu->cpumask)
bitmap = vcpu->cpumask;
...

vcpu->cpumask is "shortened" in that it is only long enough to contain the last
set bit in the mask. However, when we go to copy the mask to the buffer that is
returned, we use the masklen passed to the function which is the "full"
masklen with a bit for each cpu. So it seems virBitmapToDataBuf copies some
extra data past the end of the bitmask. Why the "192" value is always set and I
typically see similar bogus bits set is still unknown.

What is the function meant to assume in this case? Is it sane to assume that
the bitmask is the full length of the buffer here and it's the responsibility
of the setter of vcpu->cpumask to provide the length of the bitmap we're
expecting? Or should we assume that we may receive a shortened bitmask here and
expand the bitmask before copying to the buffer?

-John

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

[libvirt] [PATCH 1/7] util: buffer: Remove struct member munging

2019-03-29 Thread Peter Krempa

This was meant to stop abusing the members directly, but we don't do
this for other internal structs. Additionally this did not stop the
test from touching the members. Remove the header obscurization.

Signed-off-by: Peter Krempa 
---
 src/util/virbuffer.c | 12 
 src/util/virbuffer.h | 16 ++--
 tests/virbuftest.c   |  2 +-
 3 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 12bdd13d39..8bb9c8e1fa 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -23,24 +23,12 @@
 #include 
 #include "c-ctype.h"

-#define __VIR_BUFFER_C__
-
 #include "virbuffer.h"
 #include "virerror.h"
 #include "virstring.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

-/* If adding more fields, ensure to edit buf.h to match
-   the number of fields */
-struct _virBuffer {
-unsigned int size;
-unsigned int use;
-unsigned int error; /* errno value, or -1 for usage error */
-int indent;
-char *content;
-};
-
 /**
  * virBufferFail
  * @buf: the buffer
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index b399c90154..16cd8515d6 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -35,19 +35,15 @@
 typedef struct _virBuffer virBuffer;
 typedef virBuffer *virBufferPtr;

-# ifndef __VIR_BUFFER_C__
-#  define VIR_BUFFER_INITIALIZER { 0, 0, 0, 0, NULL }
+# define VIR_BUFFER_INITIALIZER { 0, 0, 0, 0, NULL }

-/* This struct must be kept in sync with the real struct
-   in the buf.c impl file */
 struct _virBuffer {
-unsigned int a;
-unsigned int b;
-unsigned int c;
-int d;
-char *e;
+unsigned int size;
+unsigned int use;
+unsigned int error; /* errno value, or -1 for usage error */
+int indent;
+char *content;
 };
-# endif

 const char *virBufferCurrentContent(virBufferPtr buf);
 char *virBufferContentAndReset(virBufferPtr buf);
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index 34f02b1281..b608da94d4 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -29,7 +29,7 @@ static int testBufInfiniteLoop(const void *data)
  * which was the case after the above addchar at the time of the bug.
  * This test is a bit fragile, since it relies on virBuffer internals.
  */
-if (virAsprintf(, "%*s", buf->a - buf->b - 1, "a") < 0)
+if (virAsprintf(, "%*s", buf->size - buf->use - 1, "a") < 0)
 goto out;

 if (info->doEscape)
-- 
2.20.1

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

[libvirt] [PATCH 5/7] util: json: Export virJSONValueToBuffer

2019-03-29 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 src/libvirt_private.syms | 1 +
 src/util/virjson.c   | 4 ++--
 src/util/virjson.h   | 5 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 73ef24d66f..7b9ea23ab9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2202,6 +2202,7 @@ virJSONValueObjectKeysNumber;
 virJSONValueObjectRemoveKey;
 virJSONValueObjectStealArray;
 virJSONValueObjectStealObject;
+virJSONValueToBuffer;
 virJSONValueToString;


diff --git a/src/util/virjson.c b/src/util/virjson.c
index 19857d2f2f..c519f8139e 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1970,7 +1970,7 @@ virJSONValueToStringOne(virJSONValuePtr object,
 }


-static int
+int
 virJSONValueToBuffer(virJSONValuePtr object,
  virBufferPtr buf,
  bool pretty)
@@ -2031,7 +2031,7 @@ virJSONValueFromString(const char *jsonstring 
ATTRIBUTE_UNUSED)
 }


-static int
+int
 virJSONValueToBuffer(virJSONValuePtr object ATTRIBUTE_UNUSED,
  virBufferPtr buf ATTRIBUTE_UNUSED,
  bool pretty ATTRIBUTE_UNUSED)
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 3dee103aba..ec86603794 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -26,6 +26,7 @@
 # include "internal.h"
 # include "virbitmap.h"
 # include "viralloc.h"
+# include "virbuffer.h"

 # include 

@@ -143,6 +144,10 @@ int virJSONValueArrayAppendString(virJSONValuePtr object, 
const char *value);
 virJSONValuePtr virJSONValueFromString(const char *jsonstring);
 char *virJSONValueToString(virJSONValuePtr object,
bool pretty);
+int virJSONValueToBuffer(virJSONValuePtr object,
+ virBufferPtr buf,
+ bool pretty)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;

 typedef int (*virJSONValueObjectIteratorFunc)(const char *key,
   virJSONValuePtr value,
-- 
2.20.1

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

[libvirt] [PATCH 4/7] util: json: Don't bother logging output string in virJSONValueToString

2019-03-29 Thread Peter Krempa

We have tests that validate the XML formatter. Additionally almost every
guide tells users to disable JSON logging. Drop logging of output string
in virJSONValueToString.

Signed-off-by: Peter Krempa 
---
 src/util/virjson.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index 7dfc589944..19857d2f2f 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -2048,16 +2048,11 @@ virJSONValueToString(virJSONValuePtr object,
  bool pretty)
 {
 VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-char *ret = NULL;

 if (virJSONValueToBuffer(object, , pretty) < 0)
 return NULL;

-ret = virBufferContentAndReset();
-
-VIR_DEBUG("result=%s", NULLSTR(ret));
-
-return ret;
+return virBufferContentAndReset();
 }


-- 
2.20.1

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

[libvirt] [PATCH 7/7] qemu: monitor: Avoid unnecessary copies of command string

2019-03-29 Thread Peter Krempa

Use virJSONValueToBuffer so that we can append the command terminator
string without copying of the string again. Also avoid a 'strlen' as we
can query the buffer use size.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_monitor_json.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index c7a7e3fa56..8e6c3ccd63 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -277,7 +277,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,
 {
 int ret = -1;
 qemuMonitorMessage msg;
-char *cmdstr = NULL;
+VIR_AUTOCLEAN(virBuffer) cmdbuf = VIR_BUFFER_INITIALIZER;
 char *id = NULL;

 *reply = NULL;
@@ -294,11 +294,15 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,
 }
 }

-if (!(cmdstr = virJSONValueToString(cmd, false)))
+if (virJSONValueToBuffer(cmd, , false) < 0)
 goto cleanup;
-if (virAsprintf(, "%s\r\n", cmdstr) < 0)
+virBufferAddLit(, "\r\n");
+
+if (virBufferCheckError() < 0)
 goto cleanup;
-msg.txLength = strlen(msg.txBuffer);
+
+msg.txLength = virBufferUse();
+msg.txBuffer = virBufferContentAndReset();
 msg.txFD = scm_fd;

 ret = qemuMonitorSend(mon, );
@@ -315,7 +319,6 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon,

  cleanup:
 VIR_FREE(id);
-VIR_FREE(cmdstr);
 VIR_FREE(msg.txBuffer);

 return ret;
-- 
2.20.1

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

[libvirt] [PATCH 2/7] util: buffer: Use 'size_t' for buffer size variables

2019-03-29 Thread Peter Krempa

Use size_t for all sizes. The '*' modifier unfortunately does require an
int so a temporary variable is necessary in the tests.

Signed-off-by: Peter Krempa 
---
 src/util/virbuffer.c | 2 +-
 src/util/virbuffer.h | 6 +++---
 tests/virbuftest.c   | 4 +++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 8bb9c8e1fa..2e1e4abead 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -339,7 +339,7 @@ virBufferCheckErrorInternal(const virBuffer *buf,
  *
  * Return the string usage in bytes
  */
-unsigned int
+size_t
 virBufferUse(const virBuffer *buf)
 {
 if (buf == NULL)
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index 16cd8515d6..18957ae02c 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -38,8 +38,8 @@ typedef virBuffer *virBufferPtr;
 # define VIR_BUFFER_INITIALIZER { 0, 0, 0, 0, NULL }

 struct _virBuffer {
-unsigned int size;
-unsigned int use;
+size_t size;
+size_t use;
 unsigned int error; /* errno value, or -1 for usage error */
 int indent;
 char *content;
@@ -69,7 +69,7 @@ VIR_DEFINE_AUTOCLEAN_FUNC(virBuffer, virBufferFreeAndReset);
 # define virBufferCheckError(buf) \
 virBufferCheckErrorInternal(buf, VIR_FROM_THIS, __FILE__, __FUNCTION__, \
 __LINE__)
-unsigned int virBufferUse(const virBuffer *buf);
+size_t virBufferUse(const virBuffer *buf);
 void virBufferAdd(virBufferPtr buf, const char *str, int len);
 void virBufferAddBuffer(virBufferPtr buf, virBufferPtr toadd);
 void virBufferAddChar(virBufferPtr buf, char c);
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index b608da94d4..778754d7c1 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -20,6 +20,7 @@ static int testBufInfiniteLoop(const void *data)
 char *addstr = NULL, *bufret = NULL;
 int ret = -1;
 const struct testInfo *info = data;
+int len;

 virBufferAddChar(buf, 'a');

@@ -29,7 +30,8 @@ static int testBufInfiniteLoop(const void *data)
  * which was the case after the above addchar at the time of the bug.
  * This test is a bit fragile, since it relies on virBuffer internals.
  */
-if (virAsprintf(, "%*s", buf->size - buf->use - 1, "a") < 0)
+len = buf->size - buf->use - 1;
+if (virAsprintf(, "%*s", len, "a") < 0)
 goto out;

 if (info->doEscape)
-- 
2.20.1

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

[libvirt] [PATCH 0/7] qemu: Improve JSON handling in monitor interactions

2019-03-29 Thread Peter Krempa

Remove a few unnecessary copies of the JSON string as well as duplicate
and unneeded debug logs.

Peter Krempa (7):
  util: buffer: Remove struct member munging
  util: buffer: Use 'size_t' for buffer size variables
  util: json: Use virBuffer in JSON->string conversion
  util: json: Don't bother logging output string in virJSONValueToString
  util: json: Export virJSONValueToBuffer
  qemu: monitor: Remove few debug statements
  qemu: monitor: Avoid unnecessary copies of command string

 src/libvirt_private.syms |  1 +
 src/qemu/qemu_monitor_json.c | 19 ---
 src/util/virbuffer.c | 14 +-
 src/util/virbuffer.h | 18 +++---
 src/util/virjson.c   | 33 -
 src/util/virjson.h   |  5 +
 tests/virbuftest.c   |  4 +++-
 7 files changed, 49 insertions(+), 45 deletions(-)

-- 
2.20.1

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

Re: [libvirt] [PATCH] tests: Don't use canonical paths in virstoragetest

2019-03-29 Thread Andrea Bolognani

On Fri, 2019-03-29 at 13:40 +0100, Michal Privoznik wrote:
> On 3/22/19 1:21 PM, Andrea Bolognani wrote:
> >   tests/virstoragetest.c | 50 +-
> >   1 file changed, 10 insertions(+), 40 deletions(-)
> 
> ACK and safe for freeze.

Thanks for the review :)

I'll actually wait until 5.2.0 is out before pushing it, as most
users are not going to be affected by it anyway...

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] tests: Don't use canonical paths in virstoragetest

2019-03-29 Thread Michal Privoznik


On 3/22/19 1:21 PM, Andrea Bolognani wrote:

The layout of my home directory is somewhat peculiar: I store
all git repositories in ~/src/upstream, but since I spend
almost all of my time hacking on libvirt, I also have a
convenience symlink ~/src/libvirt -> ~/src/upstream/libvirt
that I use to access that specific git repository.

The above setup has served me well for years; however, ever
since commit ca1471622dd9 dropped our own custom definitions
for abs_{,top_}{src,build}dir and started using the ones
provided by autotools, virstoragetest has started reliably
failing with errors such as

2) Storage backing chain 2 ...
   Offset 0
   Expect [chain member: 0
   path:/home/abologna/src/upstream/libvirt/tests/virstoragedata/raw
   backingStoreRaw: 
   capacity: 0
   encryption: 0
   relPath:
   type:1
   format:1
   protocol:none
   hostname:
   ]
   Actual [chain member: 0
   path:/home/abologna/src/libvirt/tests/virstoragedata/raw
   backingStoreRaw: 
   capacity: 0
   encryption: 0
   relPath:
   type:1
   format:1
   protocol:none
   hostname:
   ]
   ... FAILED

Using abolute paths instead of canonical ones in the tests makes
the problem go away.

Note that all tests that are specifically designed to test path
canonicalization via TEST_PATH_CANONICALIZE() were passing even
before this patch and are not touched by it.

Signed-off-by: Andrea Bolognani 
---

I'm far from being confident this is the correct approach, but
I've grown annoyed enough by the constant 'make check' failures
that I figured I'd at least get the discussion going :)

  tests/virstoragetest.c | 50 +-
  1 file changed, 10 insertions(+), 40 deletions(-)


ACK and safe for freeze.

Michal

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

[libvirt] [PATCH 4/6] qemu: caps: Don't leak package name string in virQEMUCapsInitQMPMonitor

2019-03-29 Thread Peter Krempa

If the detected qemu version is below our required version 'package'
would be leaked.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4c52dfc714..e3db7ce71c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4322,7 +4322,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 {
 int ret = -1;
 int major, minor, micro;
-char *package = NULL;
+VIR_AUTOFREE(char *) package = NULL;

 /* @mon is supposed to be locked by callee */

@@ -4347,7 +4347,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 }

 qemuCaps->version = major * 100 + minor * 1000 + micro;
-qemuCaps->package = package;
+VIR_STEAL_PTR(qemuCaps->package, package);
 qemuCaps->usedQMP = true;

 if (virQEMUCapsInitQMPArch(qemuCaps, mon) < 0)
-- 
2.20.1

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

[libvirt] [PATCH 1/6] qemu: caps: Separate capabilities based on qemu version

2019-03-29 Thread Peter Krempa

virQEMUCapsInitQMPMonitor is massive now since it collects calls to the
various probing functions and also version based capabilities. Split
out the version based caps into a separate function.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 124 +++
 1 file changed, 69 insertions(+), 55 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 56228e7a36..04199b1a76 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4145,6 +4145,73 @@ virQEMUCapsInitQMPBasicArch(virQEMUCapsPtr qemuCaps)
 }


+/**
+ * virQEMUCapsInitQMPVersionCaps:
+ * @qemuCaps: QEMU capabilities
+ *
+ * Add all QEMU capabilities based on version of QEMU.
+ */
+static void
+virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps)
+{
+if (qemuCaps->version >= 1006000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+
+/* vmport option is supported v2.2.0 onwards */
+if (qemuCaps->version >= 2002000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
+
+/* -cpu ...,aarch64=off supported in v2.3.0 and onwards. But it
+   isn't detectable via qmp at this point */
+if (qemuCaps->arch == VIR_ARCH_AARCH64 &&
+qemuCaps->version >= 2003000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF);
+
+/* vhost-user supports multi-queue from v2.4.0 onwards,
+ * but there is no way to query for that capability */
+if (qemuCaps->version >= 2004000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
+
+/* smm option is supported from v2.4.0 */
+if (qemuCaps->version >= 2004000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+
+/* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */
+if (qemuCaps->version >= 2004000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL);
+
+/* Since 2.4.50 ARM virt machine supports gic-version option */
+if (qemuCaps->version >= 2004050)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
+
+/* no way to query if -machine kernel_irqchip supports split */
+if (qemuCaps->version >= 2006000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT);
+
+/* HPT resizing is supported since QEMU 2.10 on ppc64; unfortunately
+ * there's no sane way to probe for it */
+if (qemuCaps->version >= 201 &&
+ARCH_IS_PPC64(qemuCaps->arch)) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
+}
+
+/* '-display egl-headless' cmdline option is supported since QEMU 2.10, but
+ * there's no way to probe it */
+if (qemuCaps->version >= 201)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_EGL_HEADLESS);
+
+/* no way to query for -numa dist */
+if (qemuCaps->version >= 201)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_NUMA_DIST);
+
+/* no way to query max-cpu-compat */
+if (qemuCaps->version >= 201 &&
+ARCH_IS_PPC64(qemuCaps->arch)) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT);
+}
+}
+
+
 static int
 virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr qemuCaps,
   qemuMonitorPtr mon)
@@ -4223,61 +4290,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,

 virQEMUCapsInitQMPBasicArch(qemuCaps);

-if (qemuCaps->version >= 1006000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
-
-/* vmport option is supported v2.2.0 onwards */
-if (qemuCaps->version >= 2002000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
-
-/* -cpu ...,aarch64=off supported in v2.3.0 and onwards. But it
-   isn't detectable via qmp at this point */
-if (qemuCaps->arch == VIR_ARCH_AARCH64 &&
-qemuCaps->version >= 2003000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF);
-
-/* vhost-user supports multi-queue from v2.4.0 onwards,
- * but there is no way to query for that capability */
-if (qemuCaps->version >= 2004000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
-
-/* smm option is supported from v2.4.0 */
-if (qemuCaps->version >= 2004000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
-
-/* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */
-if (qemuCaps->version >= 2004000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL);
-
-/* Since 2.4.50 ARM virt machine supports gic-version option */
-if (qemuCaps->version >= 2004050)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
-
-/* no way to query if -machine kernel_irqchip supports split */
-if (qemuCaps->version >= 2006000)
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT);
-
-/* HPT resizing is supported since QEMU 2.10 on ppc64; unfortunately
- * there's no sane way to probe for it */
-if (qemuCaps->version >= 201 &&
-

[libvirt] [PATCH 6/6] qemu: caps: Remove pointless debug message in virQEMUCapsInitQMPMonitor

2019-03-29 Thread Peter Krempa

Failure of qemuMonitorGetVersion is fatal now that we only support QMP
based qemus. Remove the debug message since we report an error already.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2127cfb85c..eb91295f93 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4325,13 +4325,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,

 /* @mon is supposed to be locked by callee */

-if (qemuMonitorGetVersion(mon,
-  , , ,
-  ) < 0) {
-VIR_DEBUG("Failed to query monitor version %s",
-  virGetLastErrorMessage());
+if (qemuMonitorGetVersion(mon, , , , ) < 0)
 return -1;
-}

 VIR_DEBUG("Got version %d.%d.%d (%s)",
   major, minor, micro, NULLSTR(package));
-- 
2.20.1

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

[libvirt] [PATCH 0/6] qemu: caps: Clean up virQEMUCapsInitQMPMonitor

2019-03-29 Thread Peter Krempa

Some cleanups done while attempting to implement a new capability.

Peter Krempa (6):
  qemu: caps: Separate capabilities based on qemu version
  qemu: caps: Aggregate all caps post-processing into a function
  qemu: capabilities: Move logic deciding whether to probe into probing
functions
  qemu: caps: Don't leak package name string in
virQEMUCapsInitQMPMonitor
  qemu: caps: Remove cleanup section in virQEMUCapsInitQMPMonitor
  qemu: caps: Remove pointless debug message in
virQEMUCapsInitQMPMonitor

 src/qemu/qemu_capabilities.c | 278 +++
 1 file changed, 150 insertions(+), 128 deletions(-)

-- 
2.20.1

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

[libvirt] [PATCH 2/6] qemu: caps: Aggregate all caps post-processing into a function

2019-03-29 Thread Peter Krempa

Some caps are cleared according to some more advanced logic after
detection. Split all that logic out into virQEMUCapsInitProcessCaps.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 85 +---
 1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 04199b1a76..0e48022fdb 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4212,6 +4212,55 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps)
 }


+/**
+ * virQEMUCapsInitProcessCaps:
+ * @qemuCaps: QEMU capabilities
+ *
+ * Some capability bits are enabled or disabled according to specific logic.
+ * This function collects all capability processing after the capabilities
+ * are detected.
+ */
+static void
+virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps)
+{
+/* 'intel-iommu' shows up as a device since 2.2.0, but can
+ * not be used with -device until 2.7.0. Before that it
+ * requires -machine iommu=on. So we must clear the device
+ * capability we detected on older QEMUs
+ */
+if (qemuCaps->version < 2007000 &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) {
+virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU);
+}
+
+/* Prealloc on NVDIMMs is broken on older QEMUs leading to
+ * user data corruption. If we are dealing with such version
+ * of QEMU pretend we don't know how to NVDIMM. */
+if (qemuCaps->version < 2009000 &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM))
+virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM);
+
+if (ARCH_IS_X86(qemuCaps->arch) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
+
+if (ARCH_IS_S390(qemuCaps->arch)) {
+/* Legacy assurance for QEMU_CAPS_CCW */
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_CCW);
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW_CSSID_UNRESTRICTED))
+virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW);
+}
+
+/* To avoid guest ABI regression, blockdev shall be enabled only when
+ * we are able to pass the custom 'device_id' for SCSI disks and cdroms. */
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID))
+virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV);
+}
+
+
 static int
 virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr qemuCaps,
   qemuMonitorPtr mon)
@@ -4320,17 +4369,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0)
 goto cleanup;

-/* 'intel-iommu' shows up as a device since 2.2.0, but can
- * not be used with -device until 2.7.0. Before that it
- * requires -machine iommu=on. So we must clear the device
- * capability we detected on older QEMUs
- */
-if (qemuCaps->version < 2007000 &&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) {
-virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU);
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU);
-}
-
 /* GIC capabilities, eg. available GIC versions */
 if ((qemuCaps->arch == VIR_ARCH_AARCH64 ||
  qemuCaps->arch == VIR_ARCH_ARMV6L ||
@@ -4338,26 +4376,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0)
 goto cleanup;

-/* Prealloc on NVDIMMs is broken on older QEMUs leading to
- * user data corruption. If we are dealing with such version
- * of QEMU pretend we don't know how to NVDIMM. */
-if (qemuCaps->version < 2009000 &&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM))
-virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM);
-
-if (ARCH_IS_X86(qemuCaps->arch) &&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
-
-if (ARCH_IS_S390(qemuCaps->arch)) {
-/* Legacy assurance for QEMU_CAPS_CCW */
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW) &&
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW))
-virQEMUCapsSet(qemuCaps, QEMU_CAPS_CCW);
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW_CSSID_UNRESTRICTED))
-virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW);
-}
-
 /* Probe for SEV capabilities */
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) {
 int rc = virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon);
@@ -4369,10 +4387,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST);
 }

-/* To avoid guest ABI regression, blockdev shall be enabled only when
-

[libvirt] [PATCH 5/6] qemu: caps: Remove cleanup section in virQEMUCapsInitQMPMonitor

2019-03-29 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 37 +---
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e3db7ce71c..2127cfb85c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4320,7 +4320,6 @@ int
 virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
   qemuMonitorPtr mon)
 {
-int ret = -1;
 int major, minor, micro;
 VIR_AUTOFREE(char *) package = NULL;

@@ -4331,7 +4330,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
   ) < 0) {
 VIR_DEBUG("Failed to query monitor version %s",
   virGetLastErrorMessage());
-goto cleanup;
+return -1;
 }

 VIR_DEBUG("Got version %d.%d.%d (%s)",
@@ -4343,7 +4342,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
_("QEMU version >= %d.%d.%d is required, but %d.%d.%d 
found"),
QEMU_MIN_MAJOR, QEMU_MIN_MINOR, QEMU_MIN_MICRO,
major, minor, micro);
-goto cleanup;
+return -1;
 }

 qemuCaps->version = major * 100 + minor * 1000 + micro;
@@ -4351,7 +4350,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 qemuCaps->usedQMP = true;

 if (virQEMUCapsInitQMPArch(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;

 virQEMUCapsInitQMPBasicArch(qemuCaps);

@@ -4359,40 +4358,38 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 virQEMUCapsInitQMPVersionCaps(qemuCaps);

 if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;

 /* Some capabilities may differ depending on KVM state */
 if (virQEMUCapsProbeQMPKVMState(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;

 if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPDevices(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPMachineTypes(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, false) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPTPM(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;
 if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
-goto cleanup;
+return -1;

 virQEMUCapsInitProcessCaps(qemuCaps);

-ret = 0;
- cleanup:
-return ret;
+return 0;
 }


-- 
2.20.1

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

[libvirt] [PATCH 3/6] qemu: capabilities: Move logic deciding whether to probe into probing functions

2019-03-29 Thread Peter Krempa

Most probing functions in virQEMUCapsInitQMPMonitor decide internally if
there's anything to do and return success if there isn't. Move the
decision logic for virQEMUCapsProbeQMPGICCapabilities,
virQEMUCapsProbeQMPSEVCapabilities, and virQEMUCapsProbeQMPSchemaCapabilities
into the function itself so that virQEMUCapsInitQMPMonitor looks tidy.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 47 ++--
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0e48022fdb..4c52dfc714 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2738,6 +2738,11 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr 
qemuCaps,
 virGICCapability *caps = NULL;
 int ncaps;

+if (!(qemuCaps->arch == VIR_ARCH_AARCH64 ||
+  qemuCaps->arch == VIR_ARCH_ARMV6L ||
+  qemuCaps->arch == VIR_ARCH_ARMV7L))
+return 0;
+
 if ((ncaps = qemuMonitorGetGICCapabilities(mon, )) < 0)
 return -1;

@@ -2747,7 +2752,6 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr 
qemuCaps,
 }


-/* Returns -1 on error, 0 if SEV is not supported, 1 if SEV is supported */
 static int
 virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon)
@@ -2755,12 +2759,21 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr 
qemuCaps,
 int rc = -1;
 virSEVCapability *caps = NULL;

-if ((rc = qemuMonitorGetSEVCapabilities(mon, )) <= 0)
-return rc;
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST))
+return 0;
+
+if ((rc = qemuMonitorGetSEVCapabilities(mon, )) < 0)
+return -1;
+
+/* SEV isn't actually supported */
+if (rc == 0) {
+virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST);
+return 0;
+}

 virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
 qemuCaps->sevCapabilities = caps;
-return rc;
+return 0;
 }


@@ -4270,6 +4283,9 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr 
qemuCaps,
 virHashTablePtr schema = NULL;
 size_t i;

+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA))
+return 0;
+
 if (!(schemareply = qemuMonitorQueryQMPSchema(mon)))
 return -1;

@@ -4363,29 +4379,14 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 goto cleanup;
 if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0)
 goto cleanup;
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA) &&
-virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0)
+if (virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0)
 goto cleanup;
 if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0)
 goto cleanup;
-
-/* GIC capabilities, eg. available GIC versions */
-if ((qemuCaps->arch == VIR_ARCH_AARCH64 ||
- qemuCaps->arch == VIR_ARCH_ARMV6L ||
- qemuCaps->arch == VIR_ARCH_ARMV7L) &&
-virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0)
+if (virQEMUCapsProbeQMPGICCapabilities(qemuCaps, mon) < 0)
+goto cleanup;
+if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
 goto cleanup;
-
-/* Probe for SEV capabilities */
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) {
-int rc = virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon);
-
-if (rc < 0)
-goto cleanup;
-
-if (rc == 0)
-virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST);
-}

 virQEMUCapsInitProcessCaps(qemuCaps);

-- 
2.20.1

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

Re: [libvirt] [PATCH 1/2] snapshots: allow create --redefine to force a new configuration

2019-03-29 Thread Richard Moore

> 
> On Wed, Mar 20, 2019 at 8:53 AM Peter Krempa 
> wrote:
> 
> I'm not persuaded that this workaround is necessary.
> 
> Thanks Peter for taking a deeper look!
> 
> And yes your summary seems correct.
> I personally still like to give admins the ability to force configs,
> but I'm ok if the general upstream opinion to that is no.
> 
> I have asked the reporter - on the bug that I got - to chime in here
> and
> do the "convincing" as he is the affected person I think he is more
> able
> to do so - e.g. express the pain with the suggested workaround.

> 
> -- 
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd

Hi,

I am the original fix requestor who Christian has put up with for quite
some time now! 

I will try and summarise in brief;

The initial problem was the machine type I had set in various
snapshots/VM's had been deprecated and thus rendered the snapshots all
unusable after upgrading O/S (that came packaged with newer
libvirt/qemu versions.)

I thus needed a simple, intuitive way to edit the snapshots to a
machine type that was still supported, permanently.
snapshot-edit seemed the appropriate tool for doing this, but alas it
would not, a way was needed to force it to save the changes.

One of my main reasons for using vm/snapshots is testing software
install behaviour/compatibility in Windows, so I have a variety of
Windows versions, each with snapshots taken with various levels of core
'things' installed (.net framework , C++ redistributables etc.)
Thus the snapshots could be many years old. I need to be able to flick
between them, so being able to permanently 'fix' them is my goal.

The second issue was similarly being able to edit other 'simple' facets
of the snapshots  e.g.

Attached disks, floppies or ISO's - or paths to them. 
Memory, if testing something that needed more than I ever invisaged way
back when.
MAC address (we have to register devices on the network via MAC, which
expire, and can then render the snapshot unusable if I cannot change
the MAC in the snapshot)

I hope this makes sense, I have tried to be brief, I can elaborate
further if needed.

I think being able to edit existing snapshots simply, with once
command, would be very beneficial to many people.
(I think most people trying to do this would be aware of the risks,
especially if the alternative is that the snapshot doesn’t work
anyway.)

Cheers

Richard Moore

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

Re: [libvirt] [PATCH] tests: Don't use canonical paths in virstoragetest

2019-03-29 Thread Andrea Bolognani

ping

On Fri, 2019-03-22 at 13:21 +0100, Andrea Bolognani wrote:
> The layout of my home directory is somewhat peculiar: I store
> all git repositories in ~/src/upstream, but since I spend
> almost all of my time hacking on libvirt, I also have a
> convenience symlink ~/src/libvirt -> ~/src/upstream/libvirt
> that I use to access that specific git repository.
> 
> The above setup has served me well for years; however, ever
> since commit ca1471622dd9 dropped our own custom definitions
> for abs_{,top_}{src,build}dir and started using the ones
> provided by autotools, virstoragetest has started reliably
> failing with errors such as
> 
>2) Storage backing chain 2 ...
>   Offset 0
>   Expect [chain member: 0
>   path:/home/abologna/src/upstream/libvirt/tests/virstoragedata/raw
>   backingStoreRaw: 
>   capacity: 0
>   encryption: 0
>   relPath:
>   type:1
>   format:1
>   protocol:none
>   hostname:
>   ]
>   Actual [chain member: 0
>   path:/home/abologna/src/libvirt/tests/virstoragedata/raw
>   backingStoreRaw: 
>   capacity: 0
>   encryption: 0
>   relPath:
>   type:1
>   format:1
>   protocol:none
>   hostname:
>   ]
>   ... FAILED
> 
> Using abolute paths instead of canonical ones in the tests makes
> the problem go away.
> 
> Note that all tests that are specifically designed to test path
> canonicalization via TEST_PATH_CANONICALIZE() were passing even
> before this patch and are not touched by it.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> 
> I'm far from being confident this is the correct approach, but
> I've grown annoyed enough by the constant 'make check' failures
> that I figured I'd at least get the discussion going :)
> 
>  tests/virstoragetest.c | 50 +-
>  1 file changed, 10 insertions(+), 40 deletions(-)
-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/4] qemu: introduce CHECK_STREQ_NULLABLE in qemuDomainDiskChangeSupported

2019-03-29 Thread Ján Tomko


On Thu, Mar 28, 2019 at 01:54:50PM -0400, Laine Stump wrote:

On 3/28/19 10:34 AM, Ján Tomko wrote:

A marco for comparing string fields of the disk.



Polo'ed-by: Laine Stump 


(seriously, though - s/marco/macro/ :-)




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

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_domain.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bb3a672d47..72e322d6a7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9322,6 +9322,18 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
 } \
 } while (0)
+#define CHECK_STREQ_NULLABLE(field, field_name) \
+do { \
+if (!disk->field) \
+break; \



So is a missing value in the updated XML equal to "no change"? Or Does 
a missing value actually mean "this should be un-set if it has been 
set to something"?




It is interpreted as "no change" here for all the numeric attributes
using CHECK_EQ with the last parameter set to true and all the string
attributes.

For interfaces, most of the attributes are considered as "no change"
when not present - most notably the PCI address, omitting it would
mean we use the DeviceInfo structure from the existing device until
Katerina fixed this recently:
https://bugzilla.redhat.com/show_bug.cgi?id=1599513
Thanks to this bug requesting us not to require the alias to be present:
https://bugzilla.redhat.com/show_bug.cgi?id=1621910
Michal formally documented our requirements for the virDomainUpdateDeviceFlags
API:
   The supplied XML description of the device should contain all the information
   that is found in the corresponding domain XML. Leaving out any piece of 
information
   may be treated as a request for its removal, which may be denied.

So we're consistently inconsistent here and I plan to flip a coin to
figure out whether a lack of boot order means "no change" or a "request
for removal":
https://bugzilla.redhat.com/show_bug.cgi?id=1661367

Jano



(I'm asking this because in the case of MTU for , if the 
existing interface has an mtu set (even to 1500), and the updated XML 
has no MTU, we consider that a change (and don't allow it).



Reviewed-by: Laine Stump 


once the commit message typo is fixed, and if the meaning of "not 
specified" for a field in the update truly is meant to be "don't 
change" rather than "remove any previous setting of this field and 
return it to default".




+if (STRNEQ_NULLABLE(disk->field, orig_disk->field)) { \
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \
+   _("cannot modify field '%s' of the disk"), \
+   field_name); \
+return false; \
+} \
+} while (0)
+
 CHECK_EQ(device, "device", false);
 CHECK_EQ(bus, "bus", false);
 if (STRNEQ(disk->dst, orig_disk->dst)) {
@@ -9469,6 +9481,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
 }
 #undef CHECK_EQ
+#undef CHECK_STREQ_NULLABLE
 return true;
 }





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

Re: [libvirt] [Qemu-devel] [PULL 04/14] audio: -audiodev command line option basic implementation

2019-03-29 Thread Pavel Hrdina

On Fri, Mar 29, 2019 at 11:12:55AM +0100, Markus Armbruster wrote:
> Pavel Hrdina  writes:
> 
> > On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
> >> Eric Blake  writes:
> >> 
> >> > On 3/28/19 3:06 PM, Eric Blake wrote:
> >> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
> >> >>> Markus Armbruster  writes:
> >>  Pavel Hrdina  writes:
> >> >> 
> >> > I'm glad that this is merged now and I wanted to start working on
> >> > libvirt patches, but there is one big issue with this command,
> >> > it's not introspectable by query-command-line-options.
> >> [...]
> >> > Adding Markus to CC so we can figure out how to wire up the
> >> > introspection for such command line options.
> >> [...]
> >> >>> Command line options are actually defined in qemu-options.hx, which 
> >> >>> gets
> >> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
> >> >>> the option name (without leading '-'), and whether the option takes an
> >> >>> argument.
> >> >>>
> >> >>> This is less information per option than q-c-l-o provides now.  Can we
> >> >>> add it to its output anyway without confusing existing users?
> >> [...]
> >> >>> Alternatives:
> >> [...]
> >> >>> 5. Screw it, create a new query command to return just the information
> >> >>>from qemu_options[].
> >
> > Hi Markus,
> >
> > Thanks for looking into this issue, it would be perfect to solve it
> > before QEMU 4.0 is released.
> 
> To be honest, I wouldn't bet my own money on 4.0 at this point.
> 
> I understand why you're eager to switch libvirt to -audiodev, it's such
> a massive improvement over the traditional mess.  But is it urgent?
> Does it fix bugs?  Does it add features?  Sure it can't wait for 4.1?

It's definitely not urgent, the audio situation is broken the whole
time so we can wait for 4.1.  It will help us to fix some bugs but
nothing critical.

Pavel


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

[libvirt] [PATCH 1/4] qemu: Require PCIe Root Port for PCI by default on ARM virt

2019-03-29 Thread Andrea Bolognani

Our PCIe topology depends on the availability of PCIe Root Ports,
so if none of the suitable devices (pcie-root-port, ioh3420) is
compiled into QEMU we should fall back to virtio-mmio rather than
trying to use PCI addresses only to fail immediately afterwards
when we realize we can't use the necessary controllers.

Note that this additional check is basically moot for ARM virt
guests, because PCIe Root Ports were enabled in QEMU builds for
the architecture well before guest OS support had been widely
available; however, the opposite is true for RISC-V, and tweaking
the code this way will allow us to share it between architectures.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_domain_address.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 28e79af7b1..9592dbfa60 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -473,9 +473,12 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def,
 return;
 
 /* We use virtio-mmio by default on mach-virt guests only if they already
- * have at least one virtio-mmio device: in all other cases, we prefer
- * virtio-pci */
+ * have at least one virtio-mmio device: in all other cases, assuming
+ * the QEMU binary supports all necessary capabilities (PCIe Root plus
+ * some kind of PCIe Root Port), we prefer virtio-pci */
 if (qemuDomainHasPCIeRoot(def) &&
+(virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT) ||
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) &&
 !qemuDomainHasVirtioMMIODevices(def)) {
 qemuDomainPrimeVirtioDeviceAddresses(def,
  
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
-- 
2.20.1

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

[libvirt] [PATCH 4/4] news: Document PCI by default on RISC-V

2019-03-29 Thread Andrea Bolognani

Signed-off-by: Andrea Bolognani 
---
 docs/news.xml | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2067830848..5715d0a510 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -148,6 +148,18 @@
   tables are not required.
 
   
+  
+
+  qemu: Use PCI by default for RISC-V guests
+
+
+  PCI support for RISC-V guests was already available in libvirt
+  5.1.0, but it required the user to opt-in by manually assigning
+  PCI addresses: with this release, RISC-V guests will use PCI
+  automatically when running against a recent enough (4.0.0+) QEMU
+  release.
+
+  
 
 
 
-- 
2.20.1

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

Re: [libvirt] [PATCH 1/2] snapshots: allow create --redefine to force a new configuration

2019-03-29 Thread Richard Moore

>
> On Wed, Mar 20, 2019 at 8:53 AM Peter Krempa 
> wrote:
>
> I'm not persuaded that this workaround is necessary.
>
> Thanks Peter for taking a deeper look!
>
> And yes your summary seems correct.
> I personally still like to give admins the ability to force configs,
> but I'm ok if the general upstream opinion to that is no.
>
> I have asked the reporter - on the bug that I got - to chime in here
> and
> do the "convincing" as he is the affected person I think he is more
> able
> to do so - e.g. express the pain with the suggested workaround.

>
> --
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd

Hi,

I am the original fix requestor who Christian has put up with for quite
some time now!

I will try and summarise in brief;

The initial problem was the machine type I had set in various
snapshots/VM's had been deprecated and thus rendered the snapshots all
unusable after upgrading O/S (that came packaged with newer
libvirt/qemu versions.)

I thus needed a simple, intuitive way to edit the snapshots to a
machine type that was still supported, permanently.
snapshot-edit seemed the appropriate tool for doing this, but alas it
would not, a way was needed to force it to save the changes.

One of my main reasons for using vm/snapshots is testing software
install behaviour/compatibility in Windows, so I have a variety of
Windows versions, each with snapshots taken with various levels of core
'things' installed (.net framework , C++ redistributables etc.)
Thus the snapshots could be many years old. I need to be able to flick
between them, so being able to permanently 'fix' them is my goal.

The second issue was similarly being able to edit other 'simple' facets
of the snapshots  e.g.

Attached disks, floppies or ISO's - or paths to them.
Memory, if testing something that needed more than I ever invisaged way
back when.
MAC address (we have to register devices on the network via MAC, which
expire, and can then render the snapshot unusable if I cannot change
the MAC in the snapshot)

I hope this makes sense, I have tried to be brief, I can elaborate
further if needed.

I think being able to edit existing snapshots simply, with once
command, would be very beneficial to many people.
(I think most people trying to do this would be aware of the risks,
especially if the alternative is that the snapshot doesnt work
anyway.)

Cheers

Richard Moore

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

[libvirt] [PATCH 2/4] qemu: Unify address assignment for virt guests

2019-03-29 Thread Andrea Bolognani

The rules are the same for all virt guests, regardless of the
architecture.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_domain_address.c | 38 --
 1 file changed, 9 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 9592dbfa60..2ec21e65ac 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -460,19 +460,23 @@ qemuDomainHasVirtioMMIODevices(virDomainDefPtr def)
 
 
 static void
-qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def,
-   virQEMUCapsPtr qemuCaps)
+qemuDomainAssignVirtioMMIOAddresses(virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps)
 {
 if (def->os.arch != VIR_ARCH_ARMV6L &&
 def->os.arch != VIR_ARCH_ARMV7L &&
-def->os.arch != VIR_ARCH_AARCH64)
+def->os.arch != VIR_ARCH_AARCH64 &&
+!ARCH_IS_RISCV(def->os.arch)) {
 return;
+}
 
 if (!(STRPREFIX(def->os.machine, "vexpress-") ||
-  qemuDomainIsARMVirt(def)))
+  qemuDomainIsARMVirt(def) ||
+  qemuDomainIsRISCVVirt(def))) {
 return;
+}
 
-/* We use virtio-mmio by default on mach-virt guests only if they already
+/* We use virtio-mmio by default on virt guests only if they already
  * have at least one virtio-mmio device: in all other cases, assuming
  * the QEMU binary supports all necessary capabilities (PCIe Root plus
  * some kind of PCIe Root Port), we prefer virtio-pci */
@@ -489,30 +493,6 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def,
 }
 
 
-static void
-qemuDomainAssignRISCVVirtioMMIOAddresses(virDomainDefPtr def,
- virQEMUCapsPtr qemuCaps)
-{
-if (!qemuDomainIsRISCVVirt(def))
-return;
-
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) {
-qemuDomainPrimeVirtioDeviceAddresses(def,
- 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO);
-}
-}
-
-
-static void
-qemuDomainAssignVirtioMMIOAddresses(virDomainDefPtr def,
-virQEMUCapsPtr qemuCaps)
-{
-qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps);
-
-qemuDomainAssignRISCVVirtioMMIOAddresses(def, qemuCaps);
-}
-
-
 static bool
 qemuDomainDeviceSupportZPCI(virDomainDeviceDefPtr device)
 {
-- 
2.20.1

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

[libvirt] [PATCH 0/4] qemu: Use PCI by default on RISC-V

2019-03-29 Thread Andrea Bolognani

Now that the patches necessary to enable pcie-root-port usage on
RISC-V have been merged into QEMU, we can go ahead and start using
PCI by default on such guests when appropriate.

The full series, with patch 3/4 in its unabridged form, can be
obtained from

  https://github.com/andreabolognani/libvirt/tree/riscv-pci-by-default

Andrea Bolognani (4):
  qemu: Require PCIe Root Port for PCI by default on ARM virt
  qemu: Unify address assignment for virt guests
  tests: Refresh capabilities for QEMU 4.0.0 on RISC-V
  news: Document PCI by default on RISC-V

 docs/news.xml |   12 +
 src/qemu/qemu_domain_address.c|   45 +-
 .../caps_4.0.0.riscv32.replies| 3864 
 .../caps_4.0.0.riscv32.xml|   19 +-
 .../caps_4.0.0.riscv64.replies| 3876 +
 .../caps_4.0.0.riscv64.xml|   19 +-
 .../riscv64-virt-headless.riscv64-latest.args |   20 +-
 7 files changed, 4206 insertions(+), 3649 deletions(-)

-- 
2.20.1

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

[libvirt] [PATCH 3/4] tests: Refresh capabilities for QEMU 4.0.0 on RISC-V

2019-03-29 Thread Andrea Bolognani

There are a few differences, but the one we're interested in is
that PCIe Root Ports are finally available: as a result of this,
our riscv64-virt-headless guest will switch from virtio-mmio to
virtio-pci.

Signed-off-by: Andrea Bolognani 
---

This version of the patch is heavily snipped to comply with the
libvir-list message size limitations.

 .../caps_4.0.0.riscv32.replies| 3864 
 .../caps_4.0.0.riscv32.xml|   19 +-
 .../caps_4.0.0.riscv64.replies| 3876 +
 .../caps_4.0.0.riscv64.xml|   19 +-
 .../riscv64-virt-headless.riscv64-latest.args |   20 +-
 5 files changed, 4180 insertions(+), 3618 deletions(-)

diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies 
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies
index cb51093656..c7dac44289 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies
@@ -17,11 +17,11 @@
 {
   "return": {
 "qemu": {
-  "micro": 50,
+  "micro": 91,
   "minor": 1,
   "major": 3
 },
-"package": "v3.1.0-1281-g006dce5f8f"
+"package": "v4.0.0-rc1-33-ga04d91c701"
   },
   "id": "libvirt-2"
 }
[...]
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml 
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
index 396e3019a0..6f81ff72c4 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
[...]
@@ -160,15 +166,16 @@
   
   
   
+  
   
-  3001050
+  3001091
   0
   0
-  v3.1.0-1281-g006dce5f8f
+  v4.0.0-rc1-33-ga04d91c701
   riscv32
   
   
-  
+  
   
   
 
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies 
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies
index beadeb2c02..6fda8ad2d2 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies
@@ -17,11 +17,11 @@
 {
   "return": {
 "qemu": {
-  "micro": 50,
+  "micro": 91,
   "minor": 1,
   "major": 3
 },
-"package": "v3.1.0-1281-g006dce5f8f"
+"package": "v4.0.0-rc1-33-ga04d91c701"
   },
   "id": "libvirt-2"
 }
[...]
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml 
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
index addc6ae4d3..242a851653 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
[...]
@@ -160,15 +166,16 @@
   
   
   
+  
   
-  3001050
+  3001091
   0
   0
-  v3.1.0-1281-g006dce5f8f
+  v4.0.0-rc1-33-ga04d91c701
   riscv64
   
   
-  
+  
   
   
 
diff --git a/tests/qemuxml2argvdata/riscv64-virt-headless.riscv64-latest.args 
b/tests/qemuxml2argvdata/riscv64-virt-headless.riscv64-latest.args
index 7b03aef933..53e6e3bf11 100644
--- a/tests/qemuxml2argvdata/riscv64-virt-headless.riscv64-latest.args
+++ b/tests/qemuxml2argvdata/riscv64-virt-headless.riscv64-latest.args
@@ -25,21 +25,29 @@ file=/tmp/lib/domain--1-guest/master-key.aes \
 -rtc base=utc \
 -no-shutdown \
 -boot strict=on \
--device virtio-serial-device,id=virtio-serial0 \
+-device 
pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\
+addr=0x1 \
+-device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
+-device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
+-device pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
+-device pcie-root-port,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x1.0x4 \
+-device pcie-root-port,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x1.0x5 \
+-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x0 \
 -drive file=/var/lib/libvirt/images/guest.qcow2,format=qcow2,if=none,\
 id=drive-virtio-disk0 \
--device virtio-blk-device,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,\
-bootindex=1 \
+-device virtio-blk-pci,scsi=off,bus=pci.3,addr=0x0,drive=drive-virtio-disk0,\
+id=virtio-disk0,bootindex=1 \
 -netdev user,id=hostnet0 \
--device virtio-net-device,netdev=hostnet0,id=net0,mac=52:54:00:09:a4:37 \
+-device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:09:a4:37,bus=pci.1,\
+addr=0x0 \
 -chardev pty,id=charserial0 \
 -serial chardev:charserial0 \
 -chardev socket,id=charchannel0,fd=1729,server,nowait \
 -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\
 id=channel0,name=org.qemu.guest_agent.0 \
--device virtio-balloon-device,id=balloon0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 \
 -object rng-random,id=objrng0,filename=/dev/random \
--device virtio-rng-device,rng=objrng0,id=rng0 \
+-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.5,addr=0x0 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
 -msg timestamp=on
-- 
2.20.1

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

Re: [libvirt] [Qemu-devel] [PULL 04/14] audio: -audiodev command line option basic implementation

2019-03-29 Thread Markus Armbruster

Pavel Hrdina  writes:

> On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>> > On 3/28/19 3:06 PM, Eric Blake wrote:
>> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
>> >>> Markus Armbruster  writes:
>>  Pavel Hrdina  writes:
>> >> 
>> > I'm glad that this is merged now and I wanted to start working on
>> > libvirt patches, but there is one big issue with this command,
>> > it's not introspectable by query-command-line-options.
>> [...]
>> > Adding Markus to CC so we can figure out how to wire up the
>> > introspection for such command line options.
>> [...]
>> >>> Command line options are actually defined in qemu-options.hx, which gets
>> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
>> >>> the option name (without leading '-'), and whether the option takes an
>> >>> argument.
>> >>>
>> >>> This is less information per option than q-c-l-o provides now.  Can we
>> >>> add it to its output anyway without confusing existing users?
>> [...]
>> >>> Alternatives:
>> [...]
>> >>> 5. Screw it, create a new query command to return just the information
>> >>>from qemu_options[].
>
> Hi Markus,
>
> Thanks for looking into this issue, it would be perfect to solve it
> before QEMU 4.0 is released.

To be honest, I wouldn't bet my own money on 4.0 at this point.

I understand why you're eager to switch libvirt to -audiodev, it's such
a massive improvement over the traditional mess.  But is it urgent?
Does it fix bugs?  Does it add features?  Sure it can't wait for 4.1?

>> >> Alternative 6:
>> >> 
>> >> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
>> >> 
>> >> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
>> >> 
>> >> Add a new feature: audiodev-command-line
>> >> 
>> >> That addition becomes both introspectible (since query-qemu-features
>> >> options are introspectible regardless of their runtime state) and
>> >> queryable (not that this feature needs runtime queries, but others might).
>> 
>> What is query-qemu-features about?  Is it about QMP, or is it a grab-bag
>> of ad hoc flags?  To be discussed further in the thread you quoted.
>
> From what I was reading the query-qemu-features should be used for
> features that are not introspectable from qmp-schema or by other means.
> Yes, the 'audiodev' is not currently in the schema but in order to have
> reasonable introspection we should not report only that the command line
> option is present, we should report also all parameters of that option.

Yes.  I feel pressing query-qemu-features into service here would border
on abuse.

Of course, abuse can be less bad than doing nothing.  I depends on the
particular case.

>> >> And, since we're already proposing query-qemu-features for 4.0 for
>> >> another reason, making it 2 reasons instead of 1 feels like extra
>> >> justification for getting it done in a timely manner.
>> >
>> > And answering myself after a bit more thought - the question is not just
>> > about "can we use the command line instead of envvars", but one step
>> > further of "once we are using the command line, what works in this
>> > release as opposed to added in later releases".  So we still want
>> > introspection to land on the full QAPI types for audiodev, even if, for
>> > 4.0, we can't actually use QMP to change them. This means we at least
>> > need a QMP command that references the QAPI types (even if the command
>> > is named "x-audiodev-dummy" and always fails), so that the types at
>> > least make it into the introspection output, coupled with the
>> > query-qemu-features bit to state that even when we remove the hack of
>> > the x-audiodev-dummy command later, we can still use audiodev and scrape
>> > enough out of introspection for our needs.
>> 
>> Alternative 7: a hack to make QAPI type Audiodev visible in
>> query-qmp-schema.
>> 
>> query-qmp-schema shows exactly the types reachable from QMP commands and
>> events.
>> 
>> You can't look up a type by name there, you have to start at a command
>> or an event.  We'd have to create a suitable dummy.  Recent precedence:
>> 
>> commit e1ca8f7e1915496148f6e0ce1f7c2309af013312
>> Author: Gerd Hoffmann 
>> Date:   Thu Nov 22 08:16:13 2018 +0100
>> 
>> qapi: add query-display-options command
>> 
>> Add query-display-options command, which allows querying the qemu
>> display configuration.  This isn't particularly useful, except it
>> exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
>> can discover recently added -display parameter rendernode (commit
>> d4dc4ab133b).  Works around lack of sufficiently powerful command 
>> line
>> introspection.
>> 
>> Signed-off-by: Gerd Hoffmann 
>> 
>> As much as I dislike such hacks, they can be the least bad option.
>> 
>> Compare alternative 7. to 5.: 7. exposes complete syntactic information
>> just for

[libvirt] [dockerfiles PATCH 2/2] Drop Ubuntu 16.04 Dockerfile

2019-03-29 Thread Andrea Bolognani

The libvirt project no longer considers it a target platform.

The corresponding libvirt-jenkins-ci commit is c6c648534dd1.

Signed-off-by: Andrea Bolognani 
---
 buildenv-ubuntu-16.Dockerfile | 85 ---
 1 file changed, 85 deletions(-)
 delete mode 100644 buildenv-ubuntu-16.Dockerfile

diff --git a/buildenv-ubuntu-16.Dockerfile b/buildenv-ubuntu-16.Dockerfile
deleted file mode 100644
index dc1b16f..000
--- a/buildenv-ubuntu-16.Dockerfile
+++ /dev/null
@@ -1,85 +0,0 @@
-FROM ubuntu:16.04
-
-RUN export DEBIAN_FRONTEND=noninteractive && \
-apt-get update && \
-apt-get dist-upgrade -y && \
-apt-get install --no-install-recommends -y \
-augeas-tools \
-autoconf \
-automake \
-autopoint \
-bash \
-bash-completion \
-ccache \
-chrony \
-dnsmasq-base \
-dwarves \
-ebtables \
-gcc \
-gettext \
-git \
-glusterfs-common \
-iproute2 \
-kmod \
-libacl1-dev \
-libapparmor-dev \
-libattr1-dev \
-libaudit-dev \
-libavahi-client-dev \
-libblkid-dev \
-libc-dev-bin \
-libc6-dev \
-libcap-ng-dev \
-libcurl4-gnutls-dev \
-libdbus-1-dev \
-libdevmapper-dev \
-libfuse-dev \
-libgnutls28-dev \
-libiscsi-dev \
-libnetcf-dev \
-libnl-3-dev \
-libnl-route-3-dev \
-libnuma-dev \
-libopenwsman-dev \
-libparted-dev \
-libpcap0.8-dev \
-libpciaccess-dev \
-librbd-dev \
-libreadline-dev \
-libsanlock-dev \
-libsasl2-dev \
-libselinux1-dev \
-libssh-dev \
-libssh2-1-dev \
-libtirpc-dev \
-libtool \
-libtool-bin \
-libudev-dev \
-libxen-dev \
-libxml2-dev \
-libxml2-utils \
-libyajl-dev \
-lvm2 \
-make \
-nfs-common \
-numad \
-open-iscsi \
-parted \
-patch \
-perl \
-pkgconf \
-policykit-1 \
-qemu-utils \
-radvd \
-screen \
-scrub \
-sheepdog \
-sudo \
-systemtap-sdt-dev \
-vim \
-wireshark-dev \
-xfslibs-dev \
-xsltproc \
-zfs-fuse && \
-apt-get autoremove -y && \
-apt-get autoclean -y
-- 
2.20.1

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

[libvirt] [dockerfiles PATCH 0/2] Drop obsolete Dockerfiles

2019-03-29 Thread Andrea Bolognani

The corresponding platforms are not longer supported by the
libvirt-jenkins-ci project.

Series pushed under the Dockerfile refresh rule.

Andrea Bolognani (2):
  Drop Debian 8 Dockerfile
  Drop Ubuntu 16.04 Dockerfile

 buildenv-debian-8.Dockerfile  | 82 -
 buildenv-ubuntu-16.Dockerfile | 85 ---
 2 files changed, 167 deletions(-)
 delete mode 100644 buildenv-debian-8.Dockerfile
 delete mode 100644 buildenv-ubuntu-16.Dockerfile

-- 
2.20.1

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

[libvirt] [dockerfiles PATCH 1/2] Drop Debian 8 Dockerfile

2019-03-29 Thread Andrea Bolognani

The libvirt project no longer considers it a target platform.

The corresponding libvirt-jenkins-ci commit is 170d87bc9810.

Signed-off-by: Andrea Bolognani 
---
 buildenv-debian-8.Dockerfile | 82 
 1 file changed, 82 deletions(-)
 delete mode 100644 buildenv-debian-8.Dockerfile

diff --git a/buildenv-debian-8.Dockerfile b/buildenv-debian-8.Dockerfile
deleted file mode 100644
index fa92347..000
--- a/buildenv-debian-8.Dockerfile
+++ /dev/null
@@ -1,82 +0,0 @@
-FROM debian:8
-
-RUN export DEBIAN_FRONTEND=noninteractive && \
-apt-get update && \
-apt-get dist-upgrade -y && \
-apt-get install --no-install-recommends -y \
-augeas-tools \
-autoconf \
-automake \
-autopoint \
-bash \
-bash-completion \
-ccache \
-chrony \
-dnsmasq-base \
-dwarves \
-ebtables \
-gcc \
-gettext \
-git \
-glusterfs-common \
-iproute2 \
-kmod \
-libacl1-dev \
-libapparmor-dev \
-libattr1-dev \
-libaudit-dev \
-libavahi-client-dev \
-libblkid-dev \
-libc-dev-bin \
-libc6-dev \
-libcap-ng-dev \
-libcurl4-gnutls-dev \
-libdbus-1-dev \
-libdevmapper-dev \
-libfuse-dev \
-libgnutls28-dev \
-libiscsi-dev \
-libnetcf-dev \
-libnl-3-dev \
-libnl-route-3-dev \
-libnuma-dev \
-libparted-dev \
-libpcap0.8-dev \
-libpciaccess-dev \
-librbd-dev \
-libreadline-dev \
-libsanlock-dev \
-libsasl2-dev \
-libselinux1-dev \
-libssh-gcrypt-dev \
-libssh2-1-dev \
-libtirpc-dev \
-libtool \
-libtool-bin \
-libudev-dev \
-libxen-dev \
-libxml2-dev \
-libxml2-utils \
-libyajl-dev \
-lvm2 \
-make \
-nfs-common \
-open-iscsi \
-parted \
-patch \
-perl \
-pkgconf \
-policykit-1 \
-qemu-utils \
-radvd \
-screen \
-scrub \
-sheepdog \
-sudo \
-systemtap-sdt-dev \
-vim \
-xfslibs-dev \
-xsltproc \
-zfs-fuse && \
-apt-get autoremove -y && \
-apt-get autoclean -y
-- 
2.20.1

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

Re: [libvirt] [jenkins-ci PATCH 0/2] Drop Ubuntu 16.04

2019-03-29 Thread Pavel Hrdina

On Thu, Mar 28, 2019 at 05:57:57PM +0100, Andrea Bolognani wrote:
> I'm a bit conflicted about this one: on one hand, unlike Debian 8 we
> haven't really run into any issue with Ubuntu 16.04, we can still
> build almost all projects on it, and it doesn't require any crazy
> hacks to maintain; on the other hand, perhaps the reason why we still
> claim we can build almost all projects on it is because we don't
> actually run any build on it, either on CentOS CI or on Travis CI :D
> 
> Anyway: here are the patches, let's see what other people think.
> 
> Andrea Bolognani (2):
>   Stop building on Ubuntu 16.04
>   guests: Drop Ubuntu 18.04

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [Qemu-devel] [PULL 04/14] audio: -audiodev command line option basic implementation

2019-03-29 Thread Pavel Hrdina

On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
> Eric Blake  writes:
> 
> > On 3/28/19 3:06 PM, Eric Blake wrote:
> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
> >>> Markus Armbruster  writes:
>  Pavel Hrdina  writes:
> >> 
> > I'm glad that this is merged now and I wanted to start working on
> > libvirt patches, but there is one big issue with this command,
> > it's not introspectable by query-command-line-options.
> [...]
> > Adding Markus to CC so we can figure out how to wire up the
> > introspection for such command line options.
> [...]
> >>> Command line options are actually defined in qemu-options.hx, which gets
> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
> >>> the option name (without leading '-'), and whether the option takes an
> >>> argument.
> >>>
> >>> This is less information per option than q-c-l-o provides now.  Can we
> >>> add it to its output anyway without confusing existing users?
> [...]
> >>> Alternatives:
> [...]
> >>> 5. Screw it, create a new query command to return just the information
> >>>from qemu_options[].

Hi Markus,

Thanks for looking into this issue, it would be perfect to solve it
before QEMU 4.0 is released.

> >> Alternative 6:
> >> 
> >> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
> >> 
> >> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
> >> 
> >> Add a new feature: audiodev-command-line
> >> 
> >> That addition becomes both introspectible (since query-qemu-features
> >> options are introspectible regardless of their runtime state) and
> >> queryable (not that this feature needs runtime queries, but others might).
> 
> What is query-qemu-features about?  Is it about QMP, or is it a grab-bag
> of ad hoc flags?  To be discussed further in the thread you quoted.

From what I was reading the query-qemu-features should be used for
features that are not introspectable from qmp-schema or by other means.
Yes, the 'audiodev' is not currently in the schema but in order to have
reasonable introspection we should not report only that the command line
option is present, we should report also all parameters of that option.

> >> And, since we're already proposing query-qemu-features for 4.0 for
> >> another reason, making it 2 reasons instead of 1 feels like extra
> >> justification for getting it done in a timely manner.
> >
> > And answering myself after a bit more thought - the question is not just
> > about "can we use the command line instead of envvars", but one step
> > further of "once we are using the command line, what works in this
> > release as opposed to added in later releases".  So we still want
> > introspection to land on the full QAPI types for audiodev, even if, for
> > 4.0, we can't actually use QMP to change them. This means we at least
> > need a QMP command that references the QAPI types (even if the command
> > is named "x-audiodev-dummy" and always fails), so that the types at
> > least make it into the introspection output, coupled with the
> > query-qemu-features bit to state that even when we remove the hack of
> > the x-audiodev-dummy command later, we can still use audiodev and scrape
> > enough out of introspection for our needs.
> 
> Alternative 7: a hack to make QAPI type Audiodev visible in
> query-qmp-schema.
> 
> query-qmp-schema shows exactly the types reachable from QMP commands and
> events.
> 
> You can't look up a type by name there, you have to start at a command
> or an event.  We'd have to create a suitable dummy.  Recent precedence:
> 
> commit e1ca8f7e1915496148f6e0ce1f7c2309af013312
> Author: Gerd Hoffmann 
> Date:   Thu Nov 22 08:16:13 2018 +0100
> 
> qapi: add query-display-options command
> 
> Add query-display-options command, which allows querying the qemu
> display configuration.  This isn't particularly useful, except it
> exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
> can discover recently added -display parameter rendernode (commit
> d4dc4ab133b).  Works around lack of sufficiently powerful command line
> introspection.
> 
> Signed-off-by: Gerd Hoffmann 
> 
> As much as I dislike such hacks, they can be the least bad option.
> 
> Compare alternative 7. to 5.: 7. exposes complete syntactic information
> just for -audiodev, 5. exposes rudimentary syntactic information for all
> options.  There's room for both.  Doesn't mean we should do both, of
> course.

My guess is that QEMU will introduce more command line parameters which
will be able to consume both options and JSON.  Currently we have
'blockdev', 'display' and 'audiodev' where 'blockdev' is present in the
output of 'query-qmp-schema' through the 'blockdev-add' qmp-command and
for 'display' we have dedicated command.

I would see it like alternative 7. can be used now to fix the issue
present in QEMU 4.0 that we cannot introspect

Re: [libvirt] [jenkins-ci PATCH 2/2] guests: Drop Ubuntu 18.04

2019-03-29 Thread Andrea Bolognani

On Thu, 2019-03-28 at 18:42 +0100, Fabiano Fidêncio wrote:
> [snip]
> 
> Please, just do a s/18.04/16.04 in the commit message before pushing.

Good catch! I've fixed it locally, thanks :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [jenkins-ci PATCH 0/2] Drop Ubuntu 16.04

2019-03-29 Thread Andrea Bolognani

On Thu, 2019-03-28 at 17:37 +, Daniel P. Berrangé wrote:
> On Thu, Mar 28, 2019 at 05:57:57PM +0100, Andrea Bolognani wrote:
> > I'm a bit conflicted about this one: on one hand, unlike Debian 8 we
> 
> I don't see a conflict. As your first patch says, our support policy
> has eliminated this distro version, so there's no reason to have it
> in CI.

Alright! ACKs then? O:-)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [jenkins-ci PATCH 0/2] Drop Debian 8

2019-03-29 Thread Erik Skultety

On Thu, Mar 28, 2019 at 05:26:17PM +0100, Andrea Bolognani wrote:
> See patch 1/2 and
>
>   https://www.redhat.com/archives/libvir-list/2019-March/msg01864.html
>
> for the rationale.

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH 0/3] vir*ObjListAddLocked(): Produce better error message than 'Duplicate key'

2019-03-29 Thread Michal Privoznik


On 3/19/19 2:49 PM, Michal Privoznik wrote:

*** BLURB HERE ***

Michal Prívozník (3):
   DO NOT APPLY: Simple reproducer for virDomainObjListRemove
   virDomainObjListAddLocked: Produce better error message than
 'Duplicate key'
   virNWFilterBindingObjListAddLocked: Produce better error message than
 'Duplicate key'

  src/conf/virdomainobjlist.c  | 36 +---
  src/conf/virnwfilterbindingobjlist.c | 29 +-
  2 files changed, 40 insertions(+), 25 deletions(-)



ping?

Michal

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

Re: [libvirt] Can jobs suck like qemu-pr-helper does be transfered to libvirtd?

2019-03-29 Thread Peter Krempa

On Fri, Mar 29, 2019 at 02:21:00 +, Zhangbo (Oscar) wrote:

[...]

> >>This does not play well with the fact that processes as the PR helper
> >>are always required.
> >>
> >>Merging them into libvirtd would make the VM stop until libvirtd is
> >>running again. Additionally if any of the operations require persistent
> >>kernel state as e.g. file descriptors, this would be impossible as
> >>stopping libvirtd process would close the FDs which may be then
> >>impossible to reopen properly e.g. due to state.
> >
> >Thanks! Besides these reasons above, will it weaken security if we let 
> >libvirtd do
> >these jobs? For example,
> >Such sayings, like "libvirtd would become the focus from attacking forces", 
> >make
> >sense?
> 
> If there's no security concern, then, will it be OK to add a new KVM ioctl, 
> which allows
> qemu to ask kvm module to do the high prilidged jobs?

Well there actually is security concern in qemu. Libvirt attempts to run
qemu with the least amount of privileges possible as the 'untrusted'
guest interacts directly with qemu.

That is in the end the reason 'qemu-pr-helper' exists separately.


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

Re: [libvirt] [Qemu-devel] [PULL 04/14] audio: -audiodev command line option basic implementation

2019-03-29 Thread Markus Armbruster

Eric Blake  writes:

> On 3/28/19 3:06 PM, Eric Blake wrote:
>> On 3/28/19 2:32 PM, Markus Armbruster wrote:
>>> Markus Armbruster  writes:
 Pavel Hrdina  writes:
>> 
> I'm glad that this is merged now and I wanted to start working on
> libvirt patches, but there is one big issue with this command,
> it's not introspectable by query-command-line-options.
[...]
> Adding Markus to CC so we can figure out how to wire up the
> introspection for such command line options.
[...]
>>> Command line options are actually defined in qemu-options.hx, which gets
>>> massaged into qemu_options[].  For each option, qemu_options[] gives us
>>> the option name (without leading '-'), and whether the option takes an
>>> argument.
>>>
>>> This is less information per option than q-c-l-o provides now.  Can we
>>> add it to its output anyway without confusing existing users?
[...]
>>> Alternatives:
[...]
>>> 5. Screw it, create a new query command to return just the information
>>>from qemu_options[].
>> 
>> Alternative 6:
>> 
>> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
>> 
>> Add a new feature: audiodev-command-line
>> 
>> That addition becomes both introspectible (since query-qemu-features
>> options are introspectible regardless of their runtime state) and
>> queryable (not that this feature needs runtime queries, but others might).

What is query-qemu-features about?  Is it about QMP, or is it a grab-bag
of ad hoc flags?  To be discussed further in the thread you quoted.

>> And, since we're already proposing query-qemu-features for 4.0 for
>> another reason, making it 2 reasons instead of 1 feels like extra
>> justification for getting it done in a timely manner.
>
> And answering myself after a bit more thought - the question is not just
> about "can we use the command line instead of envvars", but one step
> further of "once we are using the command line, what works in this
> release as opposed to added in later releases".  So we still want
> introspection to land on the full QAPI types for audiodev, even if, for
> 4.0, we can't actually use QMP to change them. This means we at least
> need a QMP command that references the QAPI types (even if the command
> is named "x-audiodev-dummy" and always fails), so that the types at
> least make it into the introspection output, coupled with the
> query-qemu-features bit to state that even when we remove the hack of
> the x-audiodev-dummy command later, we can still use audiodev and scrape
> enough out of introspection for our needs.

Alternative 7: a hack to make QAPI type Audiodev visible in
query-qmp-schema.

query-qmp-schema shows exactly the types reachable from QMP commands and
events.

You can't look up a type by name there, you have to start at a command
or an event.  We'd have to create a suitable dummy.  Recent precedence:

commit e1ca8f7e1915496148f6e0ce1f7c2309af013312
Author: Gerd Hoffmann 
Date:   Thu Nov 22 08:16:13 2018 +0100

qapi: add query-display-options command

Add query-display-options command, which allows querying the qemu
display configuration.  This isn't particularly useful, except it
exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
can discover recently added -display parameter rendernode (commit
d4dc4ab133b).  Works around lack of sufficiently powerful command line
introspection.

Signed-off-by: Gerd Hoffmann 

As much as I dislike such hacks, they can be the least bad option.

Compare alternative 7. to 5.: 7. exposes complete syntactic information
just for -audiodev, 5. exposes rudimentary syntactic information for all
options.  There's room for both.  Doesn't mean we should do both, of
course.

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

Re: [libvirt] [PATCH] snapashot: Improve logic of virDomainMomentMoveChildren

2019-03-29 Thread Bjoern Walk

Eric Blake  [2019-03-28, 09:05AM -0500]:
> Even though Coverity can prove that 'last' is always set if the prior
> loop executed, gcc 8.0.1 cannot:
> 
>   CC   conf/libvirt_conf_la-virdomainmomentobjlist.lo
> ../../src/conf/virdomainmomentobjlist.c: In function 
> 'virDomainMomentMoveChildren':
> ../../src/conf/virdomainmomentobjlist.c:178:19: error: 'last' may be used 
> uninitialized in this function [-Werror=maybe-uninitialized]
>  last->sibling = to->first_child;
>  ~~^
> 
> Rewrite the loop to a form that should be easier for static analysis
> to work with.
> 
> Fixes: ced0898f86bf
> Reported-by: Bjoern Walk 
> Signed-off-by: Eric Blake 
> ---
> 
> Qualifies as a build-breaker fix, but I'd like a review before pushing.

Looks good to me.


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

74 matches

Mail list logo