[libvirt] [PATCH go-xml] Add support for device sound

[zhenwei.pi]

---
 domain.go  | 11 +++
 domain_test.go | 20 
 2 files changed, 31 insertions(+)

diff --git a/domain.go b/domain.go
index de47c07..bf0b851 100644
--- a/domain.go
+++ b/domain.go
@@ -283,6 +283,16 @@ type DomainMemBalloon struct {
Address *DomainAddress `xml:"address"`
 }
 
+type DomainSoundCodec struct {
+   Type string `xml:"type,attr"`
+}
+
+type DomainSound struct {
+   Model   string`xml:"model,attr"`
+   Codec   *DomainSoundCodec `xml:"codec"`
+   Address *DomainAddress`xml:"address"`
+}
+
 type DomainDeviceList struct {
Controllers []DomainController `xml:"controller"`
Disks   []DomainDisk   `xml:"disk"`
@@ -295,6 +305,7 @@ type DomainDeviceList struct {
Videos  []DomainVideo  `xml:"video"`
Channels[]DomainChardev`xml:"channel"`
MemBalloon  *DomainMemBalloon  `xml:"memballoon"`
+   Sounds  []DomainSound  `xml:"sound"`
 }
 
 type DomainMemory struct {
diff --git a/domain_test.go b/domain_test.go
index 2c7173b..7990627 100644
--- a/domain_test.go
+++ b/domain_test.go
@@ -45,6 +45,7 @@ var ifaceAddr = Address{0, 0, 4, 0}
 var videoAddr = Address{0, 0, 5, 0}
 var fsAddr = Address{0, 0, 6, 0}
 var balloonAddr = Address{0, 0, 7, 0}
+var duplexAddr = Address{0, 0, 8, 0}
 
 var serialPort uint = 0
 
@@ -276,6 +277,21 @@ var domainTestData = []struct {
},
},
},
+   Sounds: []DomainSound{
+   DomainSound{
+   Model: "ich6",
+   Codec: {
+   Type: "duplex",
+   },
+   Address: {
+   Type: "pci",
+   Domain:   
,
+   Bus:  
,
+   Slot: 
,
+   Function: 
,
+   },
+   },
+   },
},
},
Expected: []string{
@@ -295,6 +311,10 @@ var domainTestData = []struct {
``,
`  `,
``,
+   ``,
+   `  `,
+   `  `,
+   ``,
`  `,
``,
},
-- 
2.7.4

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

Re: [libvirt] Remotable Libvirt

[Cole Robinson]

On 05/31/2017 01:00 PM, Richard W.M. Jones wrote:
> On Wed, May 31, 2017 at 09:22:41AM -0700, Peter wrote:
>> The javascript is always run in the users browser. The dbus calls or
>> system commands are sent by the javascript via a websocket to
>> cockpit-ws. It then forwards those messages on to the correct
>> cockpit-bridge. Based on the payload the bridge knows how
>> communicate with the appropriate system API. (See 
>> https://github.com/cockpit-project/cockpit/blob/master/doc/protocol.md
>> or run 'cockpit-bridge --version' for a list).
>>
>> So in keeping with the cockpit ideals cockpit-bridge should not know
>> how to start a virtual machine or link to the libvirt library. It
>> should receive a payload that when properly executed by the bridge
>> results in the machine starting. Right now this is only possible
>> using the virsh type commands that have the problems discussed
>> earlier. I think it's clear that the existing RPC is not an option.
>> So that pretty much leaves us with the choice to roll or own
>> solution or to add a more generic dbus or rest wrapper for libvirt.
> 
> Thanks - that's a bit clearer now.
> 
> I agree with others that as things stand you will need a REST or DBus
> or similar API added to libvirt.
> 

One thing to consider though is that the libvirt API is only part of the
equation. The biggest part for sure, but if cockpit wants to create and manage
libvirt XML it should eventually use libvirt-gconfig, which is just a
standalone bit of code for manipulating the XML, and possibly libosinfo and
other libvirt-* libraries. Dan's writeup here describes the idea behind the
different libraries:

https://www.berrange.com/posts/2011/11/22/introducing-the-libvirt-glib-a-mapping-of-the-libvirt-api-and-xml-to-glibgobject/
https://www.redhat.com/archives/libvir-list/2012-September/msg00325.html

Though the only ones that have had any real development live in
libvirt-glib.git: libvirt-glib, libvirt-gconfig, libvirt-gobject. And they
would likely need to be extended to fit cockpit's goals, but it's better than
writing all that XML building from scratch.

> However have you considered using gobject-introspection to generate
> new "Payload" types automatically?
> 
This is an interesting idea (though I have no clue if it's technically
feasible). But if it worked it could be used to cover
libvirt-{gobject,glib,gconfig} and libosinfo which is likely everything
cockpit will ever need for virt management.

- Cole

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

[libvirt] [PATCH V3] CI: also run tests using updated distro(s)

[claudioandre . br]

From: Claudio André 

It is possible to test libvirt using other distros in Travis via Docker;
including (but not limited to) Fedora and Ubuntu.
---
https://travis-ci.org/claudioandre/libvirt/builds/237995646

 .travis.yml| 19 ++---
 tests/travis-ci.sh | 80 ++
 2 files changed, 89 insertions(+), 10 deletions(-)
 create mode 100755 tests/travis-ci.sh

diff --git a/.travis.yml b/.travis.yml
index 5a3e765..e4d4888 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,8 +1,6 @@
 sudo: false
 language: c
 dist: precise
-compiler:
-  - gcc
 cache: ccache
 addons:
   apt:
@@ -62,15 +60,14 @@ git:
 before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install gnutls 
libgcrypt yajl gettext rpcgen ; fi
 
-# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux
-before_script:
-  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
 script:
-  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+  - tests/travis-ci.sh
 
 # Environments here are run in addition to the main environment defined above
 matrix:
   include:
+- compiler: gcc
+  dist: precise
 - compiler: clang
   dist: precise
 - compiler: clang
@@ -79,10 +76,12 @@ matrix:
   dist: trusty
 - compiler: clang
   os: osx
-  script:
-# many unit tests fail & so does syntax-check, so skip for now
-# one day we must fix it though
-- make -j3
+- services: docker
+  env: IMAGE=ubuntu:17.04 CC=gcc
+  dist: trusty
+- services: docker
+  env: IMAGE=ubuntu:17.04 CC=clang
+  dist: trusty
 
 after_failure:
   - echo 
''
diff --git a/tests/travis-ci.sh b/tests/travis-ci.sh
new file mode 100755
index 000..e63d509
--- /dev/null
+++ b/tests/travis-ci.sh
@@ -0,0 +1,80 @@
+#!/bin/bash -e
+
+function do_Install_Dependencies(){
+echo
+echo '-- Installing Dependencies --'
+
+if [[ $DISTRO = "ubuntu:17.04" ]]; then
+apt-get update -qq
+apt-get -y -qq install \
+build-essential git clang autoconf libtool libcmpicppimpl0 gettext 
\
+xsltproc autopoint libxml2-dev libncurses5-dev libreadline-dev \
+zlib1g-dev libgnutls28-dev libgcrypt11-dev libavahi-client-dev 
libsasl2-dev \
+libxen-dev lvm2 libgcrypt11-dev libparted0-dev libdevmapper-dev 
uuid-dev \
+libudev-dev libpciaccess-dev libcap-ng-dev libnl-3-dev 
libnl-route-3-dev \
+libyajl-dev libpcap0.8-dev libnuma-dev libnetcf-dev libaudit-dev \
+libxml2-utils libapparmor-dev dnsmasq-base librbd-dev 
w3c-markup-validator kmod > /dev/null
+else
+echo "Please, change 'tests/travis-ci.sh' to add the needed 
dependencies for $DISTRO."
+exit 1
+fi
+}
+
+
+function do_Show_Info(){
+echo
+echo '-- Environment --'
+echo "Running on Docker: $DISTRO"
+id
+uname -a
+
+if [[ -n $CC ]]; then
+echo
+echo '-- Compiler in use --'
+"$CC" --version
+fi
+
+echo -en 'travis_fold:start:printenv\r'
+echo '-- Environment Variables --'
+printenv
+echo -en 'travis_fold:end:printenv\r'
+}
+
+
+# --- Build and Test libvirt ---
+
+if [[ -n $IMAGE ]]; then
+# Run docker using the selected image; then build and test
+docker run --privileged --cap-add=ALL -v /lib/modules:/lib/modules \
+  -v "$(pwd)":/cwd -e CC=$CC -e DISTRO=$IMAGE "$IMAGE" sh -e -c " \
+cd /cwd; \
+tests/travis-ci.sh"
+exit $?
+fi
+
+if [[ -n $DISTRO ]]; then
+do_Install_Dependencies
+do_Show_Info
+fi
+
+# Build and test
+if [[ "$TRAVIS_OS_NAME" = "osx" ]]; then
+echo -en 'travis_fold:start:autogen\r'
+echo '-- Running ./autogen.sh --'
+# The custom PATH is just to pick up OS-X homebrew
+PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
+echo -en 'travis_fold:end:autogen\r'
+
+# many unit tests fail & so does syntax-check, so skip for now
+# one day we must fix it though
+make -j3
+else
+echo -en 'travis_fold:start:autogen\r'
+echo '-- Running ./autogen.sh --'
+./autogen.sh
+echo -en 'travis_fold:end:autogen\r'
+
+VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+fi
+
+exit $?
-- 
2.11.0

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

Re: [libvirt] Remotable Libvirt

[Richard W.M. Jones]

On Wed, May 31, 2017 at 09:22:41AM -0700, Peter wrote:
> The javascript is always run in the users browser. The dbus calls or
> system commands are sent by the javascript via a websocket to
> cockpit-ws. It then forwards those messages on to the correct
> cockpit-bridge. Based on the payload the bridge knows how
> communicate with the appropriate system API. (See 
> https://github.com/cockpit-project/cockpit/blob/master/doc/protocol.md
> or run 'cockpit-bridge --version' for a list).
> 
> So in keeping with the cockpit ideals cockpit-bridge should not know
> how to start a virtual machine or link to the libvirt library. It
> should receive a payload that when properly executed by the bridge
> results in the machine starting. Right now this is only possible
> using the virsh type commands that have the problems discussed
> earlier. I think it's clear that the existing RPC is not an option.
> So that pretty much leaves us with the choice to roll or own
> solution or to add a more generic dbus or rest wrapper for libvirt.

Thanks - that's a bit clearer now.

I agree with others that as things stand you will need a REST or DBus
or similar API added to libvirt.

However have you considered using gobject-introspection to generate
new "Payload" types automatically?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

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

Re: [libvirt] Remotable Libvirt

[Peter]

On 05/31/2017 08:52 AM, Richard W.M. Jones wrote:

On Wed, May 31, 2017 at 04:02:42PM +0100, Daniel P. Berrange wrote:

On Wed, May 31, 2017 at 03:59:10PM +0100, Richard W.M. Jones wrote:

On Thu, May 25, 2017 at 10:26:47AM -0700, Peter wrote:

The majority of cockpit is implemented in
javascript.


How about using the gobject libvirt bindings?

  https://libvirt.org/git/?p=libvirt-glib.git;a=summary

They should be usable from Javascript directly, as in the .js example
here:

  
https://libvirt.org/git/?p=libvirt-glib.git;a=tree;f=examples;h=d63d5964be2299b62140f9fd183b5cc744837d8a;hb=HEAD


They're usable from a standalone javascript interpretor, but there's no
way to use them from a client side javascript interpretor in the user's
web browser.


Hmm OK, I thought "implemented in javascript" meant they were using
server-side Javascript.

I see that cockpit runs two processes on the server (cockpit-ws and
ssh-agent).

I had a look at one of the modules in the source (pkg/systemd).  It's
running local commands (eg. "grep"), and issuing dbus calls which
AFAIK cannot be issued over the network.

Can Peter explain a bit more about the architecture of Cockpit?
Where does the Javascript run?  What JS engine runs it?

Rich.



See this paragraph from my original message.

> As a quick overview, cockpit aims to talk to existing remotable
> system APIs. Usually these API’s take the form of dbus, REST or
> executable commands. The majority of cockpit is implemented in
> javascript. There is no cockpit backend that knows how to change a
> hostname for example. The cockpit backend knows how to handle a dbus
> payload. The javascript running in the users browser knows how to use
> the systemd dbus API at org.freedesktop.hostname1 to manage the
> system hostname.

The javascript is always run in the users browser. The dbus calls or 
system commands are sent by the javascript via a websocket to 
cockpit-ws. It then forwards those messages on to the correct 
cockpit-bridge. Based on the payload the bridge knows how communicate 
with the appropriate system API. (See 
https://github.com/cockpit-project/cockpit/blob/master/doc/protocol.md 
or run 'cockpit-bridge --version' for a list).


So in keeping with the cockpit ideals cockpit-bridge should not know how 
to start a virtual machine or link to the libvirt library. It should 
receive a payload that when properly executed by the bridge results in 
the machine starting. Right now this is only possible using the virsh 
type commands that have the problems discussed earlier. I think it's 
clear that the existing RPC is not an option. So that pretty much leaves 
us with the choice to roll or own solution or to add a more generic dbus 
or rest wrapper for libvirt.


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

Re: [libvirt] [PATCH V2] CI: also run tests using updated distro(s)

[Claudio André]

Em 31/05/2017 12:38, Daniel P. Berrange escreveu:

On Wed, May 31, 2017 at 12:21:05PM -0300, claudioandre...@gmail.com wrote:

From: Claudio André 

It is possible to test libvirt using other distros in Travis via Docker;
including (but not limited to) Fedora and Ubuntu.
---
See it in action at https://travis-ci.org/claudioandre/libvirt/builds/237687907

  .travis.yml| 23 ++
  tests/travis-ci.sh | 70 ++
  2 files changed, 83 insertions(+), 10 deletions(-)
  create mode 100755 tests/travis-ci.sh

diff --git a/.travis.yml b/.travis.yml
index 5a3e765..3ed2093 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,8 +1,6 @@
  sudo: false
  language: c
  dist: precise
-compiler:
-  - gcc
  cache: ccache
  addons:
apt:
@@ -62,15 +60,14 @@ git:
  before_install:
- if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install 
gnutls libgcrypt yajl gettext rpcgen ; fi
  
-# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux

-before_script:
-  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
  script:
-  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+  - tests/travis-ci.sh
  
  # Environments here are run in addition to the main environment defined above

  matrix:
include:
+- compiler: gcc
+  dist: precise
  - compiler: clang
dist: precise
  - compiler: clang
@@ -79,10 +76,16 @@ matrix:
dist: trusty
  - compiler: clang
os: osx
-  script:
-# many unit tests fail & so does syntax-check, so skip for now
-# one day we must fix it though
-- make -j3
+- services: docker
+  env: IMAGE=ubuntu:17.04 CCO=gcc
+  dist: trusty

Why is this labelled 'trusty' if we're running 17.04 which is not 'trusty'


Two different things. Trusty is the Travis worker (the "base OS") and 
17.04 is the docker image.



Also just call the env var 'CC' rather than 'CCO' as that is
the more normal convention.


Ok, I did this to avoid to use the CC that already exists in the worker 
(base OS). Will change and test.



+  allow_failures:
+- env: IMAGE=ubuntu:17.04 CCO=gcc
+- env: IMAGE=ubuntu:17.04 CCO=clang

Can you just fix the tests instead please.


They are fixed/passing. I will remove the allow failures.




  after_failure:
- echo 
''
diff --git a/tests/travis-ci.sh b/tests/travis-ci.sh
new file mode 100755
index 000..07ec85d
--- /dev/null
+++ b/tests/travis-ci.sh
@@ -0,0 +1,70 @@
+#!/bin/bash -e
+
+function do_Install_Dependencies(){
+echo
+echo '-- Installing Dependencies --'
+
+apt-get update -qq
+apt-get -y -qq install \
+build-essential git clang autoconf libtool libcmpicppimpl0 gettext 
\
+xsltproc autopoint libxml2-dev libncurses5-dev libreadline-dev \
+zlib1g-dev libgnutls28-dev libgcrypt11-dev libavahi-client-dev 
libsasl2-dev \
+libxen-dev lvm2 libgcrypt11-dev libparted0-dev libdevmapper-dev 
uuid-dev \
+libudev-dev libpciaccess-dev libcap-ng-dev libnl-3-dev 
libnl-route-3-dev \
+libyajl-dev libpcap0.8-dev libnuma-dev libnetcf-dev libaudit-dev \
+libxml2-utils libapparmor-dev dnsmasq-base librbd-dev 
w3c-markup-validator kmod > /dev/null
+}

This isn't portable so we should at least check that we have the apt-get
binary, and print out a message about porting it to other distros...


Not sure I get the point.
- Ok, it is not portable between Ubuntu versions (e.g., libgnutls-dev x 
libgnutls28-dev)

- I can do something like this:
if [ ubuntu 17.04 ]
  ok
else
  echo "please provide the installation of the needed dependencies"
  exit 1
fi


+
+function do_Show_Info(){
+echo
+echo '-- Environment --'
+echo "Running on Docker: $DISTRO"
+id
+uname -a
+}
+
+
+function do_Show_Compiler(){
+
+if [[ -n $CC ]]; then
+echo
+echo '-- Compiler in use --'
+"$CC" --version
+fi
+}

I don't think we need separate functions for these two - just put
it all in one place.

In fact I think it is best to just run 'printenv' instead of
special casing the 'CC' env variable.


Ok, I can add the printenv, but notice I need to print the version of 
the compiler used. So, I'll keep the 'CC' env stuff.


Ok, I can merge the two functions. But, please, notice that I have 2 
because I was thinking about debugging:
- as soon as possible, show info about the docker running environment. 
Docker run is ok?;

- install dependencies. Installation was ok?;
- then, show info about the compiler version (e.g., to help you in case 
of compiler warnings).



+

+
+# --- Build and Test libvirt ---
+
+if [[ -n $IMAGE ]]; then
+# Run docker using the selected image; then build and test
+docker run --privileged --cap-add=ALL -v 

Re: [libvirt] [PATCH] Add some news items for the 3.4.0 release

[Peter Krempa]

On Wed, May 31, 2017 at 17:35:52 +0200, Martin Kletzander wrote:
> On Wed, May 31, 2017 at 04:22:07PM +0200, Peter Krempa wrote:
> > On Wed, May 31, 2017 at 16:06:48 +0200, Martin Kletzander wrote:
> > > Signed-off-by: Martin Kletzander 
> > > ---
> > > I could not be bothered to split the patches.  Also, please review
> > > whatever you know about as this is just a compilation of stuff from
> > > the git log.
> > > 
> > >  docs/news.xml | 166 
> > > +-
> > >  1 file changed, 165 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/docs/news.xml b/docs/news.xml
> > > index 649350a904d3..d34e8beae4c0 100644
> > > --- a/docs/news.xml
> > > +++ b/docs/news.xml
> > > @@ -45,6 +45,38 @@
> > 
> > [...]
> > 
> > > +  
> > > +
> > > +  Repository now has new README.md file
> > > +
> > > +
> > > +  The new file uses markdown syntax, so it looks better on
> > > +  github and possibly other web pages, but it has also more
> > > +  useful information.  The old README is now symlink to the
> > > +  new file.
> > > +
> > > +  
> > 
> > This does not seem to be a feature to me in any possible way.
> > 
> 
> I probably wanted to put it in Improvements, can that be?

I don't feel that adding that file improved anything, but yes, that is
the more appropriate place.

> 
> > > @@ -67,10 +99,142 @@
> > 
> > [...]
> > 
> > > +  
> > > +
> > > +  libxl: NUMA sibling distances are now reported in host 
> > > capabilities
> > > +
> > > +  
> > > +  
> > > +
> > > +  Support for VMDK files with version 3
> > > +
> > > +
> > > +  VMDK version 3 files are now properly detected.
> > 
> > Was this description suggested by the department of redundancy
> > department?
> > 
> 
> I'll go with just "VMDK version 3 files are now properly detected" as a 
> summary.

That's exactly what I've meant :)

> 
> 
> > > +
> > > +  
> > > +  
> > > +
> > > +  Interrupt remapping and Extended interrupt mode for IOMMU 
> > > devices
> > > +
> > > +
> > > +  These two new features can now be controlled with new
> > > +  driver intremap='on/off' eim='on/off'/
> > > +  tag for iommu devices.
> > > +
> > > +  
> > 
> > Why was the other iommu change considered a feature and this is an
> > improvement?
> > 
> 
> What other iommu change?  I can't find any in this file.


> 
> > 
> > ACK to everything excepthe readme.md section.

ACK


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

Re: [libvirt] Remotable Libvirt

[Richard W.M. Jones]

On Wed, May 31, 2017 at 04:02:42PM +0100, Daniel P. Berrange wrote:
> On Wed, May 31, 2017 at 03:59:10PM +0100, Richard W.M. Jones wrote:
> > On Thu, May 25, 2017 at 10:26:47AM -0700, Peter wrote:
> > > The majority of cockpit is implemented in
> > > javascript.
> > 
> > How about using the gobject libvirt bindings?
> > 
> >   https://libvirt.org/git/?p=libvirt-glib.git;a=summary
> > 
> > They should be usable from Javascript directly, as in the .js example
> > here:
> > 
> >   
> > https://libvirt.org/git/?p=libvirt-glib.git;a=tree;f=examples;h=d63d5964be2299b62140f9fd183b5cc744837d8a;hb=HEAD
> 
> They're usable from a standalone javascript interpretor, but there's no
> way to use them from a client side javascript interpretor in the user's
> web browser.

Hmm OK, I thought "implemented in javascript" meant they were using
server-side Javascript.

I see that cockpit runs two processes on the server (cockpit-ws and
ssh-agent).

I had a look at one of the modules in the source (pkg/systemd).  It's
running local commands (eg. "grep"), and issuing dbus calls which
AFAIK cannot be issued over the network.

Can Peter explain a bit more about the architecture of Cockpit?
Where does the Javascript run?  What JS engine runs it?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

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

Re: [libvirt] [PATCH] Add some news items for the 3.4.0 release

[Martin Kletzander]

On Wed, May 31, 2017 at 05:35:52PM +0200, Martin Kletzander wrote:

On Wed, May 31, 2017 at 04:22:07PM +0200, Peter Krempa wrote:

On Wed, May 31, 2017 at 16:06:48 +0200, Martin Kletzander wrote:

Signed-off-by: Martin Kletzander 
---
I could not be bothered to split the patches.  Also, please review
whatever you know about as this is just a compilation of stuff from
the git log.

 docs/news.xml | 166 +-
 1 file changed, 165 insertions(+), 1 deletion(-)

diff --git a/docs/news.xml b/docs/news.xml
index 649350a904d3..d34e8beae4c0 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -45,6 +45,38 @@


[...]


+  
+
+  Repository now has new README.md file
+
+
+  The new file uses markdown syntax, so it looks better on
+  github and possibly other web pages, but it has also more
+  useful information.  The old README is now symlink to the
+  new file.
+
+  


This does not seem to be a feature to me in any possible way.



I probably wanted to put it in Improvements, can that be?


@@ -67,10 +99,142 @@


[...]


+  
+
+  libxl: NUMA sibling distances are now reported in host capabilities
+
+  
+  
+
+  Support for VMDK files with version 3
+
+
+  VMDK version 3 files are now properly detected.


Was this description suggested by the department of redundancy
department?



I'll go with just "VMDK version 3 files are now properly detected" as a summary.



+
+  
+  
+
+  Interrupt remapping and Extended interrupt mode for IOMMU devices
+
+
+  These two new features can now be controlled with new
+  driver intremap='on/off' eim='on/off'/
+  tag for iommu devices.
+
+  


Why was the other iommu change considered a feature and this is an
improvement?



What other iommu change?  I can't find any in this file.



ACK to everything excepthe readme.md section.


Please, reconsider with this squashed in, since there was off-list reply
with double spaces as well:

diff --git i/docs/news.xml w/docs/news.xml
index d34e8beae4c0..b351f22990a1 100644
--- i/docs/news.xml
+++ w/docs/news.xml
@@ -57,17 +57,6 @@
  
  

-  Repository now has new README.md file
-
-
-  The new file uses markdown syntax, so it looks better on
-  github and possibly other web pages, but it has also more
-  useful information.  The old README is now symlink to the
-  new file.
-
-  
-  
-
  The reason for VM shutdown is reported, if known


@@ -81,6 +70,17 @@

  

+  Repository now has new README.md file
+
+
+  The new file uses markdown syntax, so it looks better on
+  github and possibly other web pages, but it has also more
+  useful information. The old README is now symlink to the
+  new file.
+
+  
+  
+
  qemu: Use GICv2 by default for aarch64/virt TCG guests


@@ -117,7 +117,7 @@


  If supported in the kernel, host capabilities will now list
-  L3 caches.  The code for other levels was added as well, but
+  L3 caches. The code for other levels was added as well, but
  only L3 caches are reported currently.

  
@@ -136,11 +136,8 @@
  
  

-  Support for VMDK files with version 3
+  VMDK version 3 files are now properly detected

-
-  VMDK version 3 files are now properly detected.
-
  
  

@@ -158,7 +155,7 @@


  Even though there were default addresses before this change,
-  they were not saved in the XML.  It is now possible to see
+  they were not saved in the XML. It is now possible to see
  and control the listen addresses properly.

  
@@ -169,7 +166,7 @@

  Even though they were added automatically when USB device
  was attached, they could've been missing in some other
-  cases.  The logic is now fixed so there are always USB
+  cases. The logic is now fixed so there are always USB
  controllers, even if there was none of them in the specified
  XML.

@@ -181,7 +178,7 @@

  Hitting the RPC limits we have is easier every day, so they
  were increased once again and some guessing logic was
-  improved as well.  It is now possible to get more stats than
+  improved as well. It is now possible to get more stats than
  ever using the virConnectGetAllDomainStats()
  call and push through even bigger requests and replies for
  all APIs.
@@ -195,7 +192,7 @@


 

Re: [libvirt] [PATCH v2] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Martin Kletzander]

On Wed, May 31, 2017 at 11:27:42AM -0400, Dan wrote:

On Tue, May 30, 2017 at 08:45:37AM +0200, Peter Krempa wrote:

On Mon, May 29, 2017 at 14:08:53 -0400, Daniel Liu wrote:
> The option allows someone to run domain-to-native on already existing
>  domain without the need of supplying their XML.  It is basically
>  wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> ---
>  tools/virsh-domain.c | 52 

>  1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index ccb514ef9..929f9c896 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c

[...]

> @@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
>  static bool
>  cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
>  {
> -bool ret = true;
> +bool ret = false;
>  const char *format = NULL;
> -const char *xmlFile = NULL;
> -char *configData;
> -char *xmlData;
> +const char *domain = NULL;
> +const char *xml = NULL;
> +char *xmlData = NULL;
> +char *configData = NULL;
>  unsigned int flags = 0;
>  virshControlPtr priv = ctl->privData;
> +virDomainPtr dom = NULL;
>
>  if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> +vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
> +vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)

You don't need this variable after the check below, so it's pointless to
extract it.


Right, I finally realized it this morning and deleted this variable.

>  return false;
>
> -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);

This macro is documented to work only on booleans, please don't use it
on pointers.


I switched to using VSH_EXCLUSIVE_OPTIONS(string1, string2) in patch v3;

> +
> +if (domain) {
> +if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> +return false;
> +}
> +
> +if (dom) {
> +xmlData = virDomainGetXMLDesc(dom, flags);
> +} else if (xml) {
> +if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
> +goto cleanup;
> +}
> +
> +if (!xmlData) {
> +vshError(ctl, "%s",
> + _("need either domain (ID, UUID, or name) or domain XML 
configuration file path"));

This line is very long.

>  return false;

You'll leak 'dom' here if it was looked up, but getting of the XML
failed.

Also the message is kind of wrong in that case, since you've provided a
valid name, but the XML could not be requested


Thanks a lot for the review; I was being careless.

> +}
>
> -configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, 
flags);
> -if (configData != NULL) {
> -vshPrint(ctl, "%s", configData);
> -VIR_FREE(configData);
> +if (!(configData = virConnectDomainXMLToNative(priv->conn, format, 
xmlData, flags))) {
> +vshError(ctl, "%s",
> + _("convert from domain XML to native command failed"));

For 'xen' the output is not really a command, so this message also is
not very good.


I have never used/tested xen. And actually I do not know where to start
setting it up except thinking about googling. Could you elaborate a bit
more?



For QEMU, the native interface is a command.  For Xen it is not a
command (I think it's some kind of a configuration or some other
definition).  Basically not all formats have commands as output, that's
it, nothing complicated.

There was no error before and it's still fine, so you could've just
skipped the error entirely, but that doesn't really matter.


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

Re: [libvirt] [PATCH V2] CI: also run tests using updated distro(s)

[Daniel P. Berrange]

On Wed, May 31, 2017 at 12:21:05PM -0300, claudioandre...@gmail.com wrote:
> From: Claudio André 
> 
> It is possible to test libvirt using other distros in Travis via Docker;
> including (but not limited to) Fedora and Ubuntu.
> ---
> See it in action at 
> https://travis-ci.org/claudioandre/libvirt/builds/237687907 
> 
>  .travis.yml| 23 ++
>  tests/travis-ci.sh | 70 
> ++
>  2 files changed, 83 insertions(+), 10 deletions(-)
>  create mode 100755 tests/travis-ci.sh
> 
> diff --git a/.travis.yml b/.travis.yml
> index 5a3e765..3ed2093 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,8 +1,6 @@
>  sudo: false
>  language: c
>  dist: precise
> -compiler:
> -  - gcc
>  cache: ccache
>  addons:
>apt:
> @@ -62,15 +60,14 @@ git:
>  before_install:
>- if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install 
> gnutls libgcrypt yajl gettext rpcgen ; fi
>  
> -# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux
> -before_script:
> -  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
> ./autogen.sh
>  script:
> -  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
> +  - tests/travis-ci.sh
>  
>  # Environments here are run in addition to the main environment defined above
>  matrix:
>include:
> +- compiler: gcc
> +  dist: precise
>  - compiler: clang
>dist: precise
>  - compiler: clang
> @@ -79,10 +76,16 @@ matrix:
>dist: trusty
>  - compiler: clang
>os: osx
> -  script:
> -# many unit tests fail & so does syntax-check, so skip for now
> -# one day we must fix it though
> -- make -j3
> +- services: docker
> +  env: IMAGE=ubuntu:17.04 CCO=gcc
> +  dist: trusty

Why is this labelled 'trusty' if we're running 17.04 which is not 'trusty'

Also just call the env var 'CC' rather than 'CCO' as that is
the more normal convention.

> +- services: docker
> +  env: IMAGE=ubuntu:17.04 CCO=clang
> +  dist: trusty

Again ?

> +
> +  allow_failures:
> +- env: IMAGE=ubuntu:17.04 CCO=gcc
> +- env: IMAGE=ubuntu:17.04 CCO=clang

Can you just fix the tests instead please. 

>  
>  after_failure:
>- echo 
> ''
> diff --git a/tests/travis-ci.sh b/tests/travis-ci.sh
> new file mode 100755
> index 000..07ec85d
> --- /dev/null
> +++ b/tests/travis-ci.sh
> @@ -0,0 +1,70 @@
> +#!/bin/bash -e
> +
> +function do_Install_Dependencies(){
> +echo
> +echo '-- Installing Dependencies --'
> +
> +apt-get update -qq
> +apt-get -y -qq install \
> +build-essential git clang autoconf libtool libcmpicppimpl0 
> gettext \
> +xsltproc autopoint libxml2-dev libncurses5-dev libreadline-dev \
> +zlib1g-dev libgnutls28-dev libgcrypt11-dev libavahi-client-dev 
> libsasl2-dev \
> +libxen-dev lvm2 libgcrypt11-dev libparted0-dev libdevmapper-dev 
> uuid-dev \
> +libudev-dev libpciaccess-dev libcap-ng-dev libnl-3-dev 
> libnl-route-3-dev \
> +libyajl-dev libpcap0.8-dev libnuma-dev libnetcf-dev libaudit-dev 
> \
> +libxml2-utils libapparmor-dev dnsmasq-base librbd-dev 
> w3c-markup-validator kmod > /dev/null
> +}

This isn't portable so we should at least check that we have the apt-get
binary, and print out a message about porting it to other distros...

> +
> +
> +function do_Show_Info(){
> +echo
> +echo '-- Environment --'
> +echo "Running on Docker: $DISTRO"
> +id
> +uname -a
> +}
> +
> +
> +function do_Show_Compiler(){
> +
> +if [[ -n $CC ]]; then
> +echo
> +echo '-- Compiler in use --'
> +"$CC" --version
> +fi
> +}

I don't think we need separate functions for these two - just put
it all in one place.

In fact I think it is best to just run 'printenv' instead of
special casing the 'CC' env variable.

> +
> +
> +# --- Build and Test libvirt ---
> +
> +if [[ -n $IMAGE ]]; then
> +# Run docker using the selected image; then build and test
> +docker run --privileged --cap-add=ALL -v /lib/modules:/lib/modules \
> +  -v "$(pwd)":/cwd -e CC=$CCO -e DISTRO=$IMAGE "$IMAGE" sh -e -c " \
> +cd /cwd; \
> +tests/travis-ci.sh"
> +exit $?
> +fi
> +
> +if [[ -n $DISTRO ]]; then
> +do_Show_Info
> +do_Install_Dependencies
> +do_Show_Compiler
> +fi
> +
> +echo -en 'travis_fold:start:autogen\r'
> +echo '-- Running ./autogen.sh --'
> +# The custom PATH is just to pick up OS-X homebrew & its harmless on 
> Linux
> +PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
> ./autogen.sh
> +echo -en 'travis_fold:end:autogen\r'

Per the comment, this is only needed for OS-X, so should be put inside the
the conditional for OS-X below

> +
> +# Build and test
> +if [[ 

[libvirt] [PATCH go-xml] Add support for Node Device with basic testing.

[Vladik Romanovsky]

---
 node_device.go  | 168 
 node_device_test.go | 128 +++
 2 files changed, 296 insertions(+)
 create mode 100644 node_device.go
 create mode 100644 node_device_test.go

diff --git a/node_device.go b/node_device.go
new file mode 100644
index 000..d7850e9
--- /dev/null
+++ b/node_device.go
@@ -0,0 +1,168 @@
+/*
+ * This file is part of the libvirt-go-xml project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ */
+
+package libvirtxml
+
+import (
+   "encoding/xml"
+)
+
+type NodeDevice struct {
+   Name   string `xml:"name"`
+   Path   string `xml:"path,omitempty"`
+   Parent string `xml:"parent,omitempty"`
+   Driver string `xml:"driver>name,omitempty"`
+   Capability []NodeDeviceCapability `xml:"capability"`
+}
+
+type NodeDeviceCapability struct {
+   Domain  int`xml:"domain,omitempty"`
+   Bus int`xml:"bus,omitempty"`
+   Slotint`xml:"slot,omitempty"`
+   Functionint`xml:"function,omitempty"`
+   IommuGroup  *NodeDeviceIOMMUGroup  `xml:"iommuGroup,omitempty"`
+   Numa*NodeDeviceNUMA`xml:"numa,omitempty"`
+   PciExpress  *NodeDevicePciExpress  `xml:"pci-express,omitempty"`
+   Hardware*NodeDeviceSystemHardware  `xml:"hardware"`
+   Firmware*NodeDeviceSystemFirmware  `xml:"firmware"`
+   Device  int`xml:"device"`
+   Number  int`xml:"number"`
+   Class   int`xml:"class"`
+   Subclassint`xml:"subclass"`
+   Protocolint`xml:"protocol"`
+   Description string `xml:"description,omitempty"`
+   Interface   string `xml:"interface"`
+   Address string `xml:"address"`
+   Link*NodeDeviceNetLink `xml:"link"`
+   Features[]NodeDeviceNetOffloadFeatures `xml:"feature,omitempty"`
+   UniqueIDint`xml:"unique_id"`
+   Target  int`xml:"target"`
+   Lun int`xml:"lun"`
+   Block   string `xml:"block"`
+   DriverType  string `xml:"drive_type"`
+   Model   string `xml:"model"`
+   Serial  string `xml:"serial"`
+   Sizeint`xml:"size"`
+   Hostint`xml:"host"`
+   Typestring `xml:"type"`
+   Product *NodeDeviceProduct `xml:"product,omitempty"`
+   Vendor  *NodeDeviceVendor  `xml:"vendor,omitempty"`
+   Capability  []NodeDeviceNestedCapabilities `xml:"capability,omitempty"`
+}
+
+type NodeDeviceVendor struct {
+   ID   string `xml:"id,attr,omitempty"`
+   Name string `xml:",chardata"`
+}
+type NodeDeviceProduct struct {
+   ID   string `xml:"id,attr,omitempty"`
+   Name string `xml:",chardata"`
+}
+
+type NodeDeviceNestedCapabilities struct {
+   Type   string   `xml:"type,attr"`
+   Address[]NodeDevicePCIAddress   `xml:"address,omitempty"`
+   MaxCount   int  `xml:"maxCount,attr,omitempty"`
+   VportsOPS  *NodeDeviceSCSIVportsOPS `xml:"vports_ops,omitempty"`
+   FCHost *NodeDeviceSCSIFCHost`xml:"fc_host,omitempty"`
+   MediaAvailable int 

Re: [libvirt] [PATCH] Add some news items for the 3.4.0 release

[Martin Kletzander]

On Wed, May 31, 2017 at 04:22:07PM +0200, Peter Krempa wrote:

On Wed, May 31, 2017 at 16:06:48 +0200, Martin Kletzander wrote:

Signed-off-by: Martin Kletzander 
---
I could not be bothered to split the patches.  Also, please review
whatever you know about as this is just a compilation of stuff from
the git log.

 docs/news.xml | 166 +-
 1 file changed, 165 insertions(+), 1 deletion(-)

diff --git a/docs/news.xml b/docs/news.xml
index 649350a904d3..d34e8beae4c0 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -45,6 +45,38 @@


[...]


+  
+
+  Repository now has new README.md file
+
+
+  The new file uses markdown syntax, so it looks better on
+  github and possibly other web pages, but it has also more
+  useful information.  The old README is now symlink to the
+  new file.
+
+  


This does not seem to be a feature to me in any possible way.



I probably wanted to put it in Improvements, can that be?


@@ -67,10 +99,142 @@


[...]


+  
+
+  libxl: NUMA sibling distances are now reported in host capabilities
+
+  
+  
+
+  Support for VMDK files with version 3
+
+
+  VMDK version 3 files are now properly detected.


Was this description suggested by the department of redundancy
department?



I'll go with just "VMDK version 3 files are now properly detected" as a summary.



+
+  
+  
+
+  Interrupt remapping and Extended interrupt mode for IOMMU devices
+
+
+  These two new features can now be controlled with new
+  driver intremap='on/off' eim='on/off'/
+  tag for iommu devices.
+
+  


Why was the other iommu change considered a feature and this is an
improvement?



What other iommu change?  I can't find any in this file.



ACK to everything excepthe readme.md section.


Please, reconsider with this squashed in, since there was off-list reply
with double spaces as well:

diff --git i/docs/news.xml w/docs/news.xml
index d34e8beae4c0..b351f22990a1 100644
--- i/docs/news.xml
+++ w/docs/news.xml
@@ -57,17 +57,6 @@
  
  

-  Repository now has new README.md file
-
-
-  The new file uses markdown syntax, so it looks better on
-  github and possibly other web pages, but it has also more
-  useful information.  The old README is now symlink to the
-  new file.
-
-  
-  
-
  The reason for VM shutdown is reported, if known


@@ -81,6 +70,17 @@

  

+  Repository now has new README.md file
+
+
+  The new file uses markdown syntax, so it looks better on
+  github and possibly other web pages, but it has also more
+  useful information. The old README is now symlink to the
+  new file.
+
+  
+  
+
  qemu: Use GICv2 by default for aarch64/virt TCG guests


@@ -117,7 +117,7 @@


  If supported in the kernel, host capabilities will now list
-  L3 caches.  The code for other levels was added as well, but
+  L3 caches. The code for other levels was added as well, but
  only L3 caches are reported currently.

  
@@ -136,11 +136,8 @@
  
  

-  Support for VMDK files with version 3
+  VMDK version 3 files are now properly detected

-
-  VMDK version 3 files are now properly detected.
-
  
  

@@ -158,7 +155,7 @@


  Even though there were default addresses before this change,
-  they were not saved in the XML.  It is now possible to see
+  they were not saved in the XML. It is now possible to see
  and control the listen addresses properly.

  
@@ -169,7 +166,7 @@

  Even though they were added automatically when USB device
  was attached, they could've been missing in some other
-  cases.  The logic is now fixed so there are always USB
+  cases. The logic is now fixed so there are always USB
  controllers, even if there was none of them in the specified
  XML.

@@ -181,7 +178,7 @@

  Hitting the RPC limits we have is easier every day, so they
  were increased once again and some guessing logic was
-  improved as well.  It is now possible to get more stats than
+  improved as well. It is now possible to get more stats than
  ever using the virConnectGetAllDomainStats()
  call and push through even bigger requests and replies for
  all APIs.
@@ -195,7 +192,7 @@


  It could happen that the link speed for PCIe devices was 

Re: [libvirt] [PATCH v2] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Dan]

On Tue, May 30, 2017 at 08:45:37AM +0200, Peter Krempa wrote:
> On Mon, May 29, 2017 at 14:08:53 -0400, Daniel Liu wrote:
> > The option allows someone to run domain-to-native on already existing
> >  domain without the need of supplying their XML.  It is basically
> >  wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> > ---
> >  tools/virsh-domain.c | 52 
> > 
> >  1 file changed, 40 insertions(+), 12 deletions(-)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index ccb514ef9..929f9c896 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> 
> [...]
> 
> > @@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> >  static bool
> >  cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> >  {
> > -bool ret = true;
> > +bool ret = false;
> >  const char *format = NULL;
> > -const char *xmlFile = NULL;
> > -char *configData;
> > -char *xmlData;
> > +const char *domain = NULL;
> > +const char *xml = NULL;
> > +char *xmlData = NULL;
> > +char *configData = NULL;
> >  unsigned int flags = 0;
> >  virshControlPtr priv = ctl->privData;
> > +virDomainPtr dom = NULL;
> >  
> >  if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > +vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
> > +vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> 
> You don't need this variable after the check below, so it's pointless to
> extract it.
> 
Right, I finally realized it this morning and deleted this variable.
> >  return false;
> >  
> > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
> 
> This macro is documented to work only on booleans, please don't use it
> on pointers.
> 
I switched to using VSH_EXCLUSIVE_OPTIONS(string1, string2) in patch v3;
> > +
> > +if (domain) {
> > +if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> > +return false;
> > +}
> > +
> > +if (dom) {
> > +xmlData = virDomainGetXMLDesc(dom, flags);
> > +} else if (xml) {
> > +if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
> > +goto cleanup;
> > +}
> > +
> > +if (!xmlData) {
> > +vshError(ctl, "%s",
> > + _("need either domain (ID, UUID, or name) or domain XML 
> > configuration file path"));
> 
> This line is very long.
> 
> >  return false;
> 
> You'll leak 'dom' here if it was looked up, but getting of the XML
> failed.
> 
> Also the message is kind of wrong in that case, since you've provided a
> valid name, but the XML could not be requested
> 
Thanks a lot for the review; I was being careless.
> > +}
> >  
> > -configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, 
> > flags);
> > -if (configData != NULL) {
> > -vshPrint(ctl, "%s", configData);
> > -VIR_FREE(configData);
> > +if (!(configData = virConnectDomainXMLToNative(priv->conn, format, 
> > xmlData, flags))) {
> > +vshError(ctl, "%s",
> > + _("convert from domain XML to native command failed"));
> 
> For 'xen' the output is not really a command, so this message also is
> not very good.
> 
I have never used/tested xen. And actually I do not know where to start
setting it up except thinking about googling. Could you elaborate a bit
more?

Thanks,


Dan


> > +goto cleanup;
> >  } else {
> > -ret = false;
> > +vshPrint(ctl, "%s", configData);
> > +ret = true;


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

Re: [libvirt] [PATCH] virsh: add --domain option for domain-to-native

[Dan]

On Mon, May 29, 2017 at 05:50:46PM -0300, Julio Faracco wrote:
> It would be nice to include the changes into documentation, wouldn't it?
> tools/virsh.pod has an entry for example.
> 
Many thanks for pointing it out. I modified man page by editing virsh.pod
in v3 patch.

Dan
> 2017-05-29 16:40 GMT-03:00 Dan :
> > On Mon, May 29, 2017 at 08:53:18PM +0200, Martin Kletzander wrote:
> >> On Mon, May 29, 2017 at 02:25:03PM -0400, Dan wrote:
> >> > On Tue, May 16, 2017 at 11:41:55AM +0200, Martin Kletzander wrote:
> >> > > On Mon, May 15, 2017 at 04:29:48AM -0400, Daniel Liu wrote:
> >> > > > Fix bug 835476[1].
> >> > >
> >> > > It's enough to mention it below (as you did).
> >> > >
> >> > > > virsh: add [--domain DOMAIN] option to  domxml-to-native DOMAIN 
> >> > > > COMMAND
> >> > >
> >> > > This first line is already in the subject.
> >> > >
> >> > > > Add support for the following syntax:
> >> > > > domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it 
> >> > > > supports
> >> > > > either designating domain (domain id, uuid, or name), or path to XML 
> >> > > > domain
> >> > > > configuration file.
> >> > > >
> >> > >
> >> > > I would reword this a little bit.  How would you feel about something
> >> > > along the lines of:
> >> > >
> >> > >  The option allows someone to run domain-to-native on already existing
> >> > >  domain without the need of supplying their XML.  It is basically
> >> > >  wrapper around `virsh dumpxml $dom | virsh domxml-to-native 
> >> > > /dev/stdin`.
> >> > >
> >> > >  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> >> > >
> >> > I made changes accordingly in the version 2 of this patch.
> >> > > > E.g.:
> >> > > > virsh domxml-to-native qemu-argv --domain RHEL7.3   # domain name
> >> > > > virsh domxml-to-native qemu-argv --domain 10# domain id
> >> > > > virsh domxml-to-native qemu-argv dumped_dom.xml # dumped xml
> >> > > >
> >> > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=835476
> >> > > > ---
> >> > > > tools/virsh-domain.c | 54 
> >> > > > ++--
> >> > > > 1 file changed, 44 insertions(+), 10 deletions(-)
> >> > > >
> >> > > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> >> > > > index 0d19d0e01..a79fd3ab2 100644
> >> > > > --- a/tools/virsh-domain.c
> >> > > > +++ b/tools/virsh-domain.c
> >> > > > @@ -9840,9 +9840,13 @@ static const vshCmdOptDef 
> >> > > > opts_domxmltonative[] = {
> >> > > >  .flags = VSH_OFLAG_REQ,
> >> > > >  .help = N_("target config data type format")
> >> > > > },
> >> > > > +{.name = "domain",
> >> > > > + .type = VSH_OT_DATA,
> >> > > > + .flags = VSH_OFLAG_REQ_OPT,
> >> > > > + .help = N_("domain name, id or uuid")
> >> > > > +},
> >> > > > {.name = "xml",
> >> > > >  .type = VSH_OT_DATA,
> >> > > > - .flags = VSH_OFLAG_REQ,
> >> > > >  .help = N_("xml data file to export from")
> >> > > > },
> >> > > > {.name = NULL}
> >> > > > @@ -9851,30 +9855,60 @@ static const vshCmdOptDef 
> >> > > > opts_domxmltonative[] = {
> >> > > > static bool
> >> > > > cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> >> > > > {
> >> > > > -bool ret = true;
> >> > > > +bool ret = false;
> >> > > > const char *format = NULL;
> >> > > > -const char *xmlFile = NULL;
> >> > > > -char *configData;
> >> > > > -char *xmlData;
> >> > > > +const char *domain = NULL;
> >> > > > +const char *xml = NULL;
> >> > > > +char *xmlData = NULL;
> >> > > > +char *configData = NULL;
> >> > > > unsigned int flags = 0;
> >> > > > virshControlPtr priv = ctl->privData;
> >> > > > +virDomainPtr dom = NULL;
> >> > > >
> >> > > > -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> >> > > > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> >> > >
> >> > > If this was already there, you could keep using it.  But that's not a
> >> > > big deal for me, just some others might not like it.
> >> > >
> >> > I kept the existing code and only added an additional check for "domain"
> >> > in v2, i.e.:
> >> >
> >> >if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> >> >vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
> >> >vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> >> >return false;
> >> > > > +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
> >> > > > +return false;
> >> > > > +
> >> > > > +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> >> > > > +return false;
> >> > > > +
> >> > >
> >> > > [1] So here you get the domain name/id/uuid ...
> >> > >
> >> > > > +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> >> > > > return false;
> >> > > >
> >> > > > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> >> > > > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
> >> > > > +
> >> > > > +if (domain)
> >> > > > +dom = virshCommandOptDomain(ctl, 

[libvirt] [PATCH v4 0/3] Loadparm support

[Farhan Ali]

This patch series introduces the support for new s390x 'loadparm'
feature. The 'loadparm' can be used to select the boot entry to
boot from, for a boot device.

Here is a link to the QEMU patches:
https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg00192.html

ChangeLog
-
v3 -> v4
- Break news and documentation updates to a 
  separate patch (patch 3)
- Merge test cases with qemu patch (patch 2)
- Add xml2xml test case (patch 2)
- Add qemucapabilities test (patch 2)
- Rebased the patch series on master commit
5970b13 udev: Fix build on older platforms

v2 -> v3:
- Updated news.xml and formatdomain.html.in with a more architectural
description of loadparm (patch 1)

v1 -> v2:
- Rebased the patch series on the latest master, commit
 2f69dd3 virfiletest: include linux/falloc.h

Thanks
Farhan Ali

Farhan Ali (3):
  conf : Add loadparm boot option for a boot device
  qemu : Add loadparm to qemu command line string
  news: Update news and libvirt documentation

 docs/formatdomain.html.in  | 9 +-
 docs/news.xml  |11 +
 docs/schemas/domaincommon.rng  | 7 +
 src/conf/device_conf.h | 1 +
 src/conf/domain_conf.c |69 +-
 src/qemu/qemu_capabilities.c   | 3 +
 src/qemu/qemu_capabilities.h   | 3 +
 src/qemu/qemu_command.c|33 +
 .../qemucapabilitiesdata/caps_2.9.50.s390x.replies | 14587 +++
 tests/qemucapabilitiesdata/caps_2.9.50.s390x.xml   |   302 +
 ...-machine-loadparm-multiple-disks-nets-s390.args |28 +
 ...v-machine-loadparm-multiple-disks-nets-s390.xml |43 +
 .../qemuxml2argv-machine-loadparm-net-s390.args|20 +
 .../qemuxml2argv-machine-loadparm-net-s390.xml |26 +
 ...xml2argv-machine-loadparm-s390-char-invalid.xml |26 +
 ...uxml2argv-machine-loadparm-s390-len-invalid.xml |26 +
 .../qemuxml2argv-machine-loadparm-s390.args|20 +
 .../qemuxml2argv-machine-loadparm-s390.xml |26 +
 tests/qemuxml2argvtest.c   |19 +
 ...t-machine-loadparm-multiple-disks-nets-s390.xml |44 +
 tests/qemuxml2xmltest.c| 4 +
 21 files changed, 15303 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.9.50.s390x.replies
 create mode 100644 tests/qemucapabilitiesdata/caps_2.9.50.s390x.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-machine-loadparm-multiple-disks-nets-s390.xml

-- 
1.9.1

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

[libvirt] [PATCH v4 1/3] conf : Add loadparm boot option for a boot device

[Farhan Ali]

Update the per device boot schema to add an optional loadparm parameter.

eg: 

Extend the virDomainDeviceInfo to support loadparm option.
Modify the appropriate functions to parse loadparm from boot device xml.

Signed-off-by: Farhan Ali 
Reviewed-by: Bjoern Walk 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Marc Hartmayer 
---
 docs/schemas/domaincommon.rng |  7 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c| 69 +--
 3 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4d9f8d1..ef09860 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5031,6 +5031,13 @@
   
 
   
+  
+
+  
+[a-zA-Z0-9.\s]{1,8}
+  
+
+  
   
 
   
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index a20de85..48782be 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -167,6 +167,7 @@ struct _virDomainDeviceInfo {
  * assignment, never saved and never reported.
  */
 int pciConnectFlags; /* enum virDomainPCIConnectFlags */
+char *loadparm;
 };
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c7e20b8..cd35ad2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3520,6 +3520,7 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 memset(>addr, 0, sizeof(info->addr));
 info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
 VIR_FREE(info->romfile);
+VIR_FREE(info->loadparm);
 }
 
 
@@ -5213,6 +5214,48 @@ virDomainDefValidate(virDomainDefPtr def,
 return 0;
 }
 
+/**
+ * virDomainDeviceLoadparmIsValid
+ * @loadparm : The string to validate
+ *
+ * The valid set of values for loadparm are [a-zA-Z0-9.]
+ * and blank spaces.
+ * The maximum allowed length is 8 characters.
+ * An empty string is considered invalid
+ */
+static bool
+virDomainDeviceLoadparmIsValid(const char *loadparm)
+{
+size_t i;
+
+if (virStringIsEmpty(loadparm)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("loadparm cannot be an empty string"));
+return false;
+}
+
+if (strlen(loadparm) > 8) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("loadparm '%s' exceeds 8 characters"), loadparm);
+return false;
+}
+
+for (i = 0; i < strlen(loadparm); i++) {
+uint8_t c = loadparm[i];
+
+if (('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') ||
+(c == '.') || (c == ' ')) {
+continue;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("invalid loadparm char '%c', expecting chars"
+ " in set of [a-zA-Z0-9.] and blank spaces"), c);
+return false;
+}
+}
+
+return true;
+}
 
 /* Generate a string representation of a device address
  * @info address Device address to stringify
@@ -5222,9 +5265,14 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
   virDomainDeviceInfoPtr info,
   unsigned int flags)
 {
-if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex)
-virBufferAsprintf(buf, "\n", info->bootIndex);
+if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) {
+virBufferAsprintf(buf, "bootIndex);
 
+if (info->loadparm)
+virBufferAsprintf(buf, " loadparm='%s'", info->loadparm);
+
+virBufferAddLit(buf, "/>\n");
+}
 if (info->alias &&
 !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
 virBufferAsprintf(buf, "\n", info->alias);
@@ -5650,6 +5698,7 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
 virHashTablePtr bootHash)
 {
 char *order;
+char *loadparm;
 int ret = -1;
 
 if (!(order = virXMLPropString(node, "order"))) {
@@ -5678,10 +5727,26 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
 goto cleanup;
 }
 
+loadparm = virXMLPropString(node, "loadparm");
+if (loadparm) {
+if (virStringToUpper(>loadparm, loadparm) != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to convert loadparm '%s' to upper case"),
+   loadparm);
+goto cleanup;
+}
+
+if (!virDomainDeviceLoadparmIsValid(info->loadparm)) {
+VIR_FREE(info->loadparm);
+goto cleanup;
+}
+}
+
 ret = 0;
 
  cleanup:
 VIR_FREE(order);
+VIR_FREE(loadparm);
 return ret;
 }
 
-- 
1.9.1

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

[libvirt] [PATCH v4 3/3] news: Update news and libvirt documentation

[Farhan Ali]

Update news and libvirt docs for loadparm.

Signed-off-by: Farhan Ali 
---
 docs/formatdomain.html.in |  9 +++--
 docs/news.xml | 11 +++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 07208ee..837be18 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3022,8 +3022,13 @@
   boot
   Specifies that the disk is bootable. The order
 attribute determines the order in which devices will be tried during
-boot sequence. The per-device boot elements cannot be
-used together with general boot elements in
+boot sequence. On S390 architecture only the first boot device is used.
+The optional loadparm attribute is an 8 byte parameter
+which can be queried by guests on S390 via sclp or diag 308. Linux
+guests on S390 can use loadparm to select a boot entry.
+Since 3.5.0
+The per-device boot elements cannot be used together
+with general boot elements in
 BIOS bootloader section.
 Since 0.8.8
   
diff --git a/docs/news.xml b/docs/news.xml
index 649350a..a15edb5 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,17 @@
 
   
 
+  qemu: Add support for loadparm for a boot device
+
+
+  Add an optional boot parameter 'loadparm' for a boot device.
+  Loadparm is an 8 byte parameter that, when present, is queried by
+  S390 guests via sclp or diag 308. Linux guests on S390 use it to
+  select a boot entry.
+
+  
+  
+
   Improved streams to efficiently transfer sparseness
 
 
-- 
1.9.1

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

[libvirt] [PATCH V2] CI: also run tests using updated distro(s)

[claudioandre . br]

From: Claudio André 

It is possible to test libvirt using other distros in Travis via Docker;
including (but not limited to) Fedora and Ubuntu.
---
See it in action at https://travis-ci.org/claudioandre/libvirt/builds/237687907 

 .travis.yml| 23 ++
 tests/travis-ci.sh | 70 ++
 2 files changed, 83 insertions(+), 10 deletions(-)
 create mode 100755 tests/travis-ci.sh

diff --git a/.travis.yml b/.travis.yml
index 5a3e765..3ed2093 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,8 +1,6 @@
 sudo: false
 language: c
 dist: precise
-compiler:
-  - gcc
 cache: ccache
 addons:
   apt:
@@ -62,15 +60,14 @@ git:
 before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update && brew install gnutls 
libgcrypt yajl gettext rpcgen ; fi
 
-# the custom PATH is just to pick up OS-X homebrew & its harmless on Linux
-before_script:
-  - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
 script:
-  - VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+  - tests/travis-ci.sh
 
 # Environments here are run in addition to the main environment defined above
 matrix:
   include:
+- compiler: gcc
+  dist: precise
 - compiler: clang
   dist: precise
 - compiler: clang
@@ -79,10 +76,16 @@ matrix:
   dist: trusty
 - compiler: clang
   os: osx
-  script:
-# many unit tests fail & so does syntax-check, so skip for now
-# one day we must fix it though
-- make -j3
+- services: docker
+  env: IMAGE=ubuntu:17.04 CCO=gcc
+  dist: trusty
+- services: docker
+  env: IMAGE=ubuntu:17.04 CCO=clang
+  dist: trusty
+
+  allow_failures:
+- env: IMAGE=ubuntu:17.04 CCO=gcc
+- env: IMAGE=ubuntu:17.04 CCO=clang
 
 after_failure:
   - echo 
''
diff --git a/tests/travis-ci.sh b/tests/travis-ci.sh
new file mode 100755
index 000..07ec85d
--- /dev/null
+++ b/tests/travis-ci.sh
@@ -0,0 +1,70 @@
+#!/bin/bash -e
+
+function do_Install_Dependencies(){
+echo
+echo '-- Installing Dependencies --'
+
+apt-get update -qq
+apt-get -y -qq install \
+build-essential git clang autoconf libtool libcmpicppimpl0 gettext 
\
+xsltproc autopoint libxml2-dev libncurses5-dev libreadline-dev \
+zlib1g-dev libgnutls28-dev libgcrypt11-dev libavahi-client-dev 
libsasl2-dev \
+libxen-dev lvm2 libgcrypt11-dev libparted0-dev libdevmapper-dev 
uuid-dev \
+libudev-dev libpciaccess-dev libcap-ng-dev libnl-3-dev 
libnl-route-3-dev \
+libyajl-dev libpcap0.8-dev libnuma-dev libnetcf-dev libaudit-dev \
+libxml2-utils libapparmor-dev dnsmasq-base librbd-dev 
w3c-markup-validator kmod > /dev/null
+}
+
+
+function do_Show_Info(){
+echo
+echo '-- Environment --'
+echo "Running on Docker: $DISTRO"
+id
+uname -a
+}
+
+
+function do_Show_Compiler(){
+
+if [[ -n $CC ]]; then
+echo
+echo '-- Compiler in use --'
+"$CC" --version
+fi
+}
+
+
+# --- Build and Test libvirt ---
+
+if [[ -n $IMAGE ]]; then
+# Run docker using the selected image; then build and test
+docker run --privileged --cap-add=ALL -v /lib/modules:/lib/modules \
+  -v "$(pwd)":/cwd -e CC=$CCO -e DISTRO=$IMAGE "$IMAGE" sh -e -c " \
+cd /cwd; \
+tests/travis-ci.sh"
+exit $?
+fi
+
+if [[ -n $DISTRO ]]; then
+do_Show_Info
+do_Install_Dependencies
+do_Show_Compiler
+fi
+
+echo -en 'travis_fold:start:autogen\r'
+echo '-- Running ./autogen.sh --'
+# The custom PATH is just to pick up OS-X homebrew & its harmless on Linux
+PATH="/usr/local/opt/gettext/bin:/usr/local/opt/rpcgen/bin:$PATH" 
./autogen.sh
+echo -en 'travis_fold:end:autogen\r'
+
+# Build and test
+if [[ "$TRAVIS_OS_NAME" = "osx" ]]; then
+# many unit tests fail & so does syntax-check, so skip for now
+# one day we must fix it though
+make -j3
+else
+VIR_TEST_DEBUG=1 make -j3 && make -j3 syntax-check && make -j3 check
+fi
+
+exit $?
-- 
2.11.0

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

Re: [libvirt] [PATCH v2] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Dan]

On Wed, May 31, 2017 at 02:32:51PM +0200, Martin Kletzander wrote:
> On Mon, May 29, 2017 at 02:08:53PM -0400, Daniel Liu wrote:
> > The option allows someone to run domain-to-native on already existing
> > domain without the need of supplying their XML.  It is basically
> > wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
> > ---
> > tools/virsh-domain.c | 52 
> > 
> > 1 file changed, 40 insertions(+), 12 deletions(-)
> > 
> 
> Just few things I forgot to sent, so they remained in my drafts folder.
> 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index ccb514ef9..929f9c896 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> > static bool
> > cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> > {
> > -bool ret = true;
> > +bool ret = false;
> > const char *format = NULL;
> > -const char *xmlFile = NULL;
> > -char *configData;
> > -char *xmlData;
> > +const char *domain = NULL;
> > +const char *xml = NULL;
> > +char *xmlData = NULL;
> > +char *configData = NULL;
> > unsigned int flags = 0;
> > virshControlPtr priv = ctl->privData;
> > +virDomainPtr dom = NULL;
> > 
> > if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > +vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
> > +vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> 
> You don't have to do this, you don't use the domain name anywhere, just
> the information whether it's present or not.
> 
> > return false;
> > 
> > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
> > +
> > +if (domain) {
> > +if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> > +return false;
> 
> 
> And instead of this, you can just do:
> 
> if (vshCommandOptBool("domain") &&
>(!(dom = virshCommandOptDomain(ctl, cmd, NULL
>return false;
> 
Oh yeah, I basically did what you suggested, it took me the entire
morning to figure out those things. I wish I checked your email earlier.

I was using nested-if here, but in the v3 PATCH, I took your
approach.
> > +}
> > +
> > +if (dom) {
> > +xmlData = virDomainGetXMLDesc(dom, flags);
> 
> xmlData can be NULL after this in case of an error.
> 
> > +} else if (xml) {
> > +if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
> > +goto cleanup;
> > +}
> > +
> > +if (!xmlData) {
> 
> So this could just be else {} of the last condition and if xmlData is
> NULL after calling virDomainGetXMLDesc(), there should be different
> error message.
> 
> Other things were pointed out already, I guess.
All right thanks a lot.

Dan


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

[libvirt] [PATCH v3] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Daniel Liu]

The option allows someone to run domain-to-native on already existing
 domain without the need of supplying their XML.  It is basically
 wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
---
 tools/virsh-domain.c | 53 +++-
 tools/virsh.pod  |  7 ---
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ccb514ef9..db92c4478 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9848,9 +9848,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("target config data type format")
 },
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ_OPT,
+ .help = N_("domain name, id or uuid")
+},
 {.name = "xml",
  .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
  .help = N_("xml data file to export from")
 },
 {.name = NULL}
@@ -9859,30 +9863,53 @@ static const vshCmdOptDef opts_domxmltonative[] = {
 static bool
 cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
 {
-bool ret = true;
+bool ret = false;
 const char *format = NULL;
-const char *xmlFile = NULL;
-char *configData;
-char *xmlData;
+const char *xml = NULL;
+char *xmlData = NULL;
+char *configData = NULL;
 unsigned int flags = 0;
 virshControlPtr priv = ctl->privData;
+virDomainPtr dom = NULL;
 
 if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
-vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
+vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
 return false;
 
-if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
-return false;
+VSH_EXCLUSIVE_OPTIONS("domain", "xml");
 
-configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, 
flags);
-if (configData != NULL) {
-vshPrint(ctl, "%s", configData);
-VIR_FREE(configData);
+if (vshCommandOptBool(cmd, "domain") &&
+(!(dom = virshCommandOptDomain(ctl, cmd, NULL
+return false;
+
+if (dom) {
+xmlData = virDomainGetXMLDesc(dom, flags);
+} else if (xml) {
+if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
+goto cleanup;
 } else {
-ret = false;
+vshError(ctl, "%s", _("need either domain or domain XML"));
+goto cleanup;
+}
+
+if (!xmlData) {
+vshError(ctl, "%s", _("failed to retrieve XML"));
+goto cleanup;
+}
+
+if (!(configData = virConnectDomainXMLToNative(priv->conn, format, 
xmlData, flags))) {
+vshError(ctl, "%s",
+ _("convert from domain XML to native command failed"));
+goto cleanup;
+} else {
+vshPrint(ctl, "%s", configData);
+ret = true;
 }
 
+ cleanup:
+virshDomainFree(dom);
 VIR_FREE(xmlData);
+VIR_FREE(configData);
 return ret;
 }
 
diff --git a/tools/virsh.pod b/tools/virsh.pod
index aee964689..049c2f3c7 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1433,10 +1433,11 @@ the I argument must be B. For Xen 
hypervisor, the
 I argument may be B, B, or B. For
 LXC hypervisor, the I argument must be B.
 
-=item B I I
+=item B I
+{ I<--domain> I | [I<--xml>] I }
 
-Convert the file I in domain XML format to the native guest
-configuration format named by I. For QEMU/KVM hypervisor,
+Convert the file I in domain XML format or existing domain to the
+native guest configuration format named by I. For QEMU/KVM hypervisor,
 the I argument must be B. For Xen hypervisor, the
 I argument may be B, B, or B. For
 LXC hypervisor, the I argument must be B.
-- 
2.13.0

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

Re: [libvirt] [PATCH v3 2/2] virQEMUDriverDomainABIStability: Check for memoryBacking

[Peter Krempa]

On Fri, May 26, 2017 at 11:58:45 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1450349
> 
> Problem is, qemu fails to load guest memory image if these
> attribute change on migration/restore from an image.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/libvirt_private.syms |  2 ++
>  src/qemu/qemu_conf.c |  2 +-
>  src/qemu/qemu_domain.c   | 22 ++
>  src/qemu/qemu_domain.h   |  1 +
>  4 files changed, 26 insertions(+), 1 deletion(-)

ACK. Obviously both patches are NOT safe for freeze.


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

Re: [libvirt] [PATCH v3 1/2] virDomainXMLOption: Introduce virDomainABIStabilityDomain

[Peter Krempa]

On Fri, May 26, 2017 at 11:58:44 +0200, Michal Privoznik wrote:
> While checking for ABI stability, drivers might pose additional
> checks that are not valid for general case. For instance, qemu
> driver might check some memory backing attributes because of how
> qemu works. But those attributes may work well in other drivers.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/domain_conf.c| 19 ---
>  src/conf/domain_conf.h| 16 ++--
>  src/conf/snapshot_conf.c  |  3 ++-
>  src/conf/snapshot_conf.h  |  1 +
>  src/libxl/libxl_conf.c|  2 +-
>  src/libxl/libxl_domain.c  |  4 +++-
>  src/lxc/lxc_conf.c|  3 ++-
>  src/openvz/openvz_driver.c|  2 +-
>  src/phyp/phyp_driver.c|  2 +-
>  src/qemu/qemu_capabilities.c  |  2 +-
>  src/qemu/qemu_conf.c  |  3 ++-
>  src/qemu/qemu_domain.c|  1 +
>  src/qemu/qemu_driver.c|  5 +++--
>  src/security/virt-aa-helper.c |  2 +-
>  src/test/test_driver.c|  6 --
>  src/uml/uml_driver.c  |  2 +-
>  src/vbox/vbox_common.c|  2 +-
>  src/vmware/vmware_driver.c|  2 +-
>  src/vmx/vmx.c |  2 +-
>  src/vz/vz_driver.c|  2 +-
>  src/xen/xen_driver.c  |  2 +-
>  src/xenapi/xenapi_driver.c|  2 +-
>  tests/bhyveargv2xmltest.c |  3 ++-
>  tests/qemuargv2xmltest.c  |  2 +-
>  tests/qemuxml2argvtest.c  |  2 +-
>  tests/sexpr2xmltest.c |  2 +-
>  tests/testutils.c |  4 ++--
>  tests/vmx2xmltest.c   |  2 +-
>  tests/xlconfigtest.c  |  2 +-
>  tests/xmconfigtest.c  |  2 +-
>  tests/xml2sexprtest.c |  2 +-
>  tests/xml2vmxtest.c   |  2 +-
>  32 files changed, 72 insertions(+), 36 deletions(-)

[...]

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 83e067269..446b117b7 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2537,9 +2537,19 @@ struct _virDomainXMLPrivateDataCallbacks {
>  virDomainXMLPrivateDataParseFunc  parse;
>  };
>  

Could you please document the expected return value meanings here.

> +typedef bool (*virDomainABIStabilityDomain)(const virDomainDef *src,
> +const virDomainDef *dst);
> +
> +typedef struct _virDomainABIStability virDomainABIStability;
> +typedef virDomainABIStability *virDomainABIStabilityPtr;
> +struct _virDomainABIStability {
> +virDomainABIStabilityDomain domain;
> +};

ACK


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

Re: [libvirt] Remotable Libvirt

[Daniel P. Berrange]

On Wed, May 31, 2017 at 03:59:10PM +0100, Richard W.M. Jones wrote:
> On Thu, May 25, 2017 at 10:26:47AM -0700, Peter wrote:
> > The majority of cockpit is implemented in
> > javascript.
> 
> How about using the gobject libvirt bindings?
> 
>   https://libvirt.org/git/?p=libvirt-glib.git;a=summary
> 
> They should be usable from Javascript directly, as in the .js example
> here:
> 
>   
> https://libvirt.org/git/?p=libvirt-glib.git;a=tree;f=examples;h=d63d5964be2299b62140f9fd183b5cc744837d8a;hb=HEAD

They're usable from a standalone javascript interpretor, but there's no
way to use them from a client side javascript interpretor in the user's
web browser.

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] CI: also run tests using updated distro(s)

[Claudio André]

Em 31/05/2017 04:12, Martin Kletzander escreveu:



Without -qq is much worse. We can:
1. collapse as seen above;
2. redirect stdout to /dev/null (I mean, there is no important
information here).

I prefer (and use) #2.



either -qq collapsed or just put it to /dev/null.  I don't care which
one, if there is a problem with the installation it will be in stderr
anyway.


The silent_if_passes() has some drawbacks.



Sure, I didn't know you can collapse part of the output of one command
and that was the first idea that came to my mind.

So the link shows the output with the current patch?


This is the output of the current patch: 
https://travis-ci.org/claudioandre/libvirt/builds/237687907

- everything passes (including Ubuntu 17.04)
- folded 'autogen'
- apt-get redirecting stdout to dev/null
- precise (gcc + clang) - (note: libvirt master builds only for clang)
- trusty (gcc + clang)
- OSX with more than 1 lines (note: this already happens in libvirt 
master)


So, you now have 2+2+1+2 builds. The current patch follows ASAP.

Thank you
Claudio

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

Re: [libvirt] Remotable Libvirt

[Richard W.M. Jones]

On Thu, May 25, 2017 at 10:26:47AM -0700, Peter wrote:
> The majority of cockpit is implemented in
> javascript.

How about using the gobject libvirt bindings?

  https://libvirt.org/git/?p=libvirt-glib.git;a=summary

They should be usable from Javascript directly, as in the .js example
here:

  
https://libvirt.org/git/?p=libvirt-glib.git;a=tree;f=examples;h=d63d5964be2299b62140f9fd183b5cc744837d8a;hb=HEAD

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

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

Re: [libvirt] [PATCH] virsh: ignore readline -Wstrict-prototypes warning

[Ján Tomko]

On Fri, May 26, 2017 at 09:49:15PM +0400, Roman Bogorodskiy wrote:

When building with clang 4.0.0, virsh build fails like this:

gmake[3]: Entering directory '/usr/home/novel/code/libvirt/tools'
 CC   virsh-virsh.o
In file included from virsh.c:45:
In file included from /usr/local/include/readline/readline.h:31:
/usr/local/include/readline/rltypedefs.h:35:22: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
typedef int Function () __attribute__ ((deprecated));
^
 void
/usr/local/include/readline/rltypedefs.h:36:24: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
typedef void VFunction () __attribute__ ((deprecated));
  ^
   void
/usr/local/include/readline/rltypedefs.h:37:26: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
typedef char *CPFunction () __attribute__ ((deprecated));
^
 void
/usr/local/include/readline/rltypedefs.h:38:28: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
typedef char **CPPFunction () __attribute__ ((deprecated));
  ^
   void
In file included from virsh.c:45:
/usr/local/include/readline/readline.h:385:23: error: this function declaration 
is not a prototype [-Werror,-Wstrict-prototypes]
extern int rl_message ();
 ^
  void
5 errors generated.
gmake[3]: *** [Makefile:2823: virsh-virsh.o] Error 1

Fix that by adding VIR_WARNINGS_NO_STRICT_PROTOTYPES macro that uses
"GCC diagnostic push" to ignore -Wstrict-prototypes and wrapping
readline includes with it and VIR_WARNINGS_RESET.

Bug report on the readline mailing list:

http://lists.gnu.org/archive/html/bug-readline/2017-05/msg4.html
---
src/internal.h | 4 
tools/virsh.c  | 2 ++
tools/virt-admin.c | 2 ++
3 files changed, 8 insertions(+)



ACK

Jan


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

Re: [libvirt] [PATCH] virsh: ignore readline -Wstrict-prototypes warning

[Roman Bogorodskiy]

  Roman Bogorodskiy wrote:

> When building with clang 4.0.0, virsh build fails like this:
> 
> gmake[3]: Entering directory '/usr/home/novel/code/libvirt/tools'
>   CC   virsh-virsh.o
> In file included from virsh.c:45:
> In file included from /usr/local/include/readline/readline.h:31:
> /usr/local/include/readline/rltypedefs.h:35:22: error: this function 
> declaration is not a prototype [-Werror,-Wstrict-prototypes]
> typedef int Function () __attribute__ ((deprecated));
>  ^
>   void
> /usr/local/include/readline/rltypedefs.h:36:24: error: this function 
> declaration is not a prototype [-Werror,-Wstrict-prototypes]
> typedef void VFunction () __attribute__ ((deprecated));
>^
> void
> /usr/local/include/readline/rltypedefs.h:37:26: error: this function 
> declaration is not a prototype [-Werror,-Wstrict-prototypes]
> typedef char *CPFunction () __attribute__ ((deprecated));
>  ^
>   void
> /usr/local/include/readline/rltypedefs.h:38:28: error: this function 
> declaration is not a prototype [-Werror,-Wstrict-prototypes]
> typedef char **CPPFunction () __attribute__ ((deprecated));
>^
> void
> In file included from virsh.c:45:
> /usr/local/include/readline/readline.h:385:23: error: this function 
> declaration is not a prototype [-Werror,-Wstrict-prototypes]
> extern int rl_message ();
>   ^
>void
> 5 errors generated.
> gmake[3]: *** [Makefile:2823: virsh-virsh.o] Error 1
> 
> Fix that by adding VIR_WARNINGS_NO_STRICT_PROTOTYPES macro that uses
> "GCC diagnostic push" to ignore -Wstrict-prototypes and wrapping
> readline includes with it and VIR_WARNINGS_RESET.
> 
> Bug report on the readline mailing list:
> 
>  http://lists.gnu.org/archive/html/bug-readline/2017-05/msg4.html
> ---
>  src/internal.h | 4 
>  tools/virsh.c  | 2 ++
>  tools/virt-admin.c | 2 ++
>  3 files changed, 8 insertions(+)

ping?

Roman Bogorodskiy


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

Re: [libvirt] [PATCH] Add some news items for the 3.4.0 release

[Peter Krempa]

On Wed, May 31, 2017 at 16:06:48 +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander 
> ---
> I could not be bothered to split the patches.  Also, please review
> whatever you know about as this is just a compilation of stuff from
> the git log.
> 
>  docs/news.xml | 166 
> +-
>  1 file changed, 165 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/news.xml b/docs/news.xml
> index 649350a904d3..d34e8beae4c0 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -45,6 +45,38 @@

[...]

> +  
> +
> +  Repository now has new README.md file
> +
> +
> +  The new file uses markdown syntax, so it looks better on
> +  github and possibly other web pages, but it has also more
> +  useful information.  The old README is now symlink to the
> +  new file.
> +
> +  

This does not seem to be a feature to me in any possible way.

> @@ -67,10 +99,142 @@

[...]

> +  
> +
> +  libxl: NUMA sibling distances are now reported in host capabilities
> +
> +  
> +  
> +
> +  Support for VMDK files with version 3
> +
> +
> +  VMDK version 3 files are now properly detected.

Was this description suggested by the department of redundancy
department?

> +
> +  
> +  
> +
> +  Interrupt remapping and Extended interrupt mode for IOMMU devices
> +
> +
> +  These two new features can now be controlled with new
> +  driver intremap='on/off' eim='on/off'/
> +  tag for iommu devices.
> +
> +  

Why was the other iommu change considered a feature and this is an
improvement?


ACK to everything excepthe readme.md section.


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

[libvirt] [PATCH] Add some news items for the 3.4.0 release

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
I could not be bothered to split the patches.  Also, please review
whatever you know about as this is just a compilation of stuff from
the git log.

 docs/news.xml | 166 +-
 1 file changed, 165 insertions(+), 1 deletion(-)

diff --git a/docs/news.xml b/docs/news.xml
index 649350a904d3..d34e8beae4c0 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -45,6 +45,38 @@
   sparseness.
 
   
+  
+
+  I/O APIC type can be specified for QEMU/KVM domains
+
+
+  The ioapic tag was added to domain
+  features, so the type of the I/O APIC can now
+  be specified (e.g. putting it in userspace for KVM domains).
+
+  
+  
+
+  Repository now has new README.md file
+
+
+  The new file uses markdown syntax, so it looks better on
+  github and possibly other web pages, but it has also more
+  useful information.  The old README is now symlink to the
+  new file.
+
+  
+  
+
+  The reason for VM shutdown is reported, if known
+
+
+  QEMU 2.10 will be able to report the reason for shutting
+  down (whether that was caused by the guest or not), and
+  libvirt is prepared for that and reports that information in
+  its shutdown event as well, if it is known.
+
+  
 
 
   
@@ -67,10 +99,142 @@
   is for CCW devices, most common on the S390 architecture. The second
   is for fibre channel-backed SCSI devices and exposes the
   fc_remote_port sub-capability to SCSI target devices.
-   
+
+  
+  
+
+  Node devices now report Mediated device capabilities
+
+
+  Endpoint devices support new mdev capability
+  and their parents now report the supported types in new
+  mdev_types capability.
+
+  
+  
+
+  Capabilities now report information about host caches
+
+
+  If supported in the kernel, host capabilities will now list
+  L3 caches.  The code for other levels was added as well, but
+  only L3 caches are reported currently.
+
+  
+  
+
+  POWER9 CPU model was added
+
+
+  It is now properly reported in host capabilities.
+
+  
+  
+
+  libxl: NUMA sibling distances are now reported in host capabilities
+
+  
+  
+
+  Support for VMDK files with version 3
+
+
+  VMDK version 3 files are now properly detected.
+
+  
+  
+
+  Interrupt remapping and Extended interrupt mode for IOMMU devices
+
+
+  These two new features can now be controlled with new
+  driver intremap='on/off' eim='on/off'/
+  tag for iommu devices.
+
+  
+  
+
+  Graphics in libxl domains now have default addresses
+
+
+  Even though there were default addresses before this change,
+  they were not saved in the XML.  It is now possible to see
+  and control the listen addresses properly.
+
+  
+  
+
+  Default USB controllers are now added for devices in libxl domains
+
+
+  Even though they were added automatically when USB device
+  was attached, they could've been missing in some other
+  cases.  The logic is now fixed so there are always USB
+  controllers, even if there was none of them in the specified
+  XML.
+
+  
+  
+
+  Limits for RPC messages were increased
+
+
+  Hitting the RPC limits we have is easier every day, so they
+  were increased once again and some guessing logic was
+  improved as well.  It is now possible to get more stats than
+  ever using the virConnectGetAllDomainStats()
+  call and push through even bigger requests and replies for
+  all APIs.
+
   
 
 
+  
+
+  PCIe 4.0 cards now report proper link speeds
+
+
+  It could happen that the link speed for PCIe devices was not
+  properly reported or the nodedev-dumpxml just failed.  That
+  was due to mistake in the field width, but should now work
+  properly.
+
+  
+  
+
+  qemu: Do not report errors on shutdown
+
+
+  For some users, in some rare cases, it could happen that
+  there was an error message "internal error: End of file from
+  qemu monitor" in the logs even though no problem happened.
+  The detection of 

Re: [libvirt] [PATCH] qemu: mkdir memory_backing_dir on startup

[Peter Krempa]

On Wed, May 31, 2017 at 15:17:24 +0200, Michal Privoznik wrote:
> On 05/31/2017 03:00 PM, Andrea Bolognani wrote:
> > On Sat, 2017-05-27 at 12:45 +0200, Michal Privoznik wrote:
> >> In 48d9e6cdcc and friends we've allowed users to back guest
> >> memory by a file inside the host. And in order to keep things
> >> manageable the memory_backing_dir variable was introduced to
> >> qemu.conf to specify the directory where the files are kept.
> >> However, libvirt's policy is that directories are created on
> >> domain startup if they don't exist. We've missed this one.
> >>  
> >> Signed-off-by: Michal Privoznik 
> >> ---
> >>   src/qemu/qemu_driver.c | 12 
> >>   1 file changed, 12 insertions(+)
> > 
> > ACK and safe for freeze.
> > 
> > Wouldn't mind it if you provided a very brief entry in the
> > release notes, too ;)
> > 
> 
> Ah, good point. Keep forgetting about those. Fixed and pushed. Thanks!

Please note that we've explicitly stated that changes to the news.xml
file should be in separate patches, which you've ignored.


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

Re: [libvirt] [PATCH v4 0/5] Hyperv-method invocation

[Matthias Bolte]

2017-05-19 22:57 GMT+02:00 Sri Ramanujam :
> Changes from v3:
>
> * Feedback from code review
> * Added 5 minute timeout to hypervInvokeMethod
>
> Sri Ramanujam (5):
>   hyperv: Functions to work with invocation parameters.
>   hyperv: Generate object property type information.
>   hyperv: add hypervInvokeMethod
>   hyperv: support virDomainSendKey
>   hyperv: Add support for virDomainSetMemory
>
>  src/hyperv/hyperv_driver.c| 201 
>  src/hyperv/hyperv_wmi.c   | 894 
> ++
>  src/hyperv/hyperv_wmi.h   |  93 +++-
>  src/hyperv/hyperv_wmi_classes.h   |  19 +
>  src/hyperv/hyperv_wmi_generator.input | 116 +
>  src/hyperv/hyperv_wmi_generator.py|  15 +-
>  src/hyperv/openwsman.h|   4 +
>  7 files changed, 1340 insertions(+), 2 deletions(-)

This series is missing a patch for the docs/news.xml announcing the
two new driver functions.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] [PATCH] qemu: mkdir memory_backing_dir on startup

[Michal Privoznik]

On 05/31/2017 03:00 PM, Andrea Bolognani wrote:
> On Sat, 2017-05-27 at 12:45 +0200, Michal Privoznik wrote:
>> In 48d9e6cdcc and friends we've allowed users to back guest
>> memory by a file inside the host. And in order to keep things
>> manageable the memory_backing_dir variable was introduced to
>> qemu.conf to specify the directory where the files are kept.
>> However, libvirt's policy is that directories are created on
>> domain startup if they don't exist. We've missed this one.
>>  
>> Signed-off-by: Michal Privoznik 
>> ---
>>   src/qemu/qemu_driver.c | 12 
>>   1 file changed, 12 insertions(+)
> 
> ACK and safe for freeze.
> 
> Wouldn't mind it if you provided a very brief entry in the
> release notes, too ;)
> 

Ah, good point. Keep forgetting about those. Fixed and pushed. Thanks!

Michal

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

Re: [libvirt] [PATCH 0/2] Two simple sparse streams fixes

[Pino Toscano]

On Wednesday, 31 May 2017 13:03:38 CEST Martin Kletzander wrote:
>  - vol-download --sparse --offset $source_file_size --length 1
>/path/to/source.file destination.file
> 
> - Every now and then (not always) it gets stuck waiting for the
>   daemon to receive data (see backtrace below), but the daemon is not
>   waiting for anything, it's just some weird race.  We can try
>   debugging it with wireshark later.  That file ends with a hole.
> 
> Thread 1 (Thread 0x7f1d2b434880 (LWP 28584)):
> #0  0x7f1d2796efbd in poll () at ../sysdeps/unix/syscall-template.S:84
> #1  0x7f1d2a806ee3 in poll (__timeout=5000, __nfds=2, 
> __fds=0x7ffe9effd640) at /usr/include/bits/poll2.h:46
> #2  virNetClientIOEventLoop (client=client@entry=0x563525bb06d0, 
> thiscall=thiscall@entry=0x563525badc00) at rpc/virnetclient.c:1664
> #3  0x7f1d2a8074d3 in virNetClientIO (client=client@entry=0x563525bb06d0, 
> thiscall=0x563525badc00) at rpc/virnetclient.c:1957
> #4  0x7f1d2a80780e in virNetClientSendInternal 
> (client=client@entry=0x563525bb06d0, msg=msg@entry=0x563525bb03d0, 
> expectReply=expectReply@entry=true, nonBlock=nonBlock@entry=false) at 
> rpc/virnetclient.c:2132
> #5  0x7f1d2a808dfc in virNetClientSendWithReplyStream 
> (client=client@entry=0x563525bb06d0, msg=msg@entry=0x563525bb03d0, 
> st=st@entry=0x563525bade10) at rpc/virnetclient.c:2236
> #6  0x7f1d2a80ab2d in virNetClientStreamRecvPacket 
> (st=st@entry=0x563525bade10, client=0x563525bb06d0, 
> data=data@entry=0x7f1d20686010 "", nbytes=nbytes@entry=262120, 
> nonblock=false, flags=32766, flags@entry=1) at rpc/virnetclientstream.c:499
> #7  0x7f1d2a7e0e3e in remoteStreamRecvFlags (st=0x563525badc60, 
> data=0x7f1d20686010 "", nbytes=262120, flags=1) at remote/remote_driver.c:5664
> #8  0x7f1d2a7c8347 in virStreamRecvFlags 
> (stream=stream@entry=0x563525badc60, data=0x7f1d20686010 "", 
> nbytes=nbytes@entry=262120, flags=flags@entry=1) at libvirt-stream.c:361
> #9  0x7f1d2a7c9b7f in virStreamSparseRecvAll 
> (stream=stream@entry=0x563525badc60, handler=0x563525760196 
> , holeHandler=0x56352576020b , 
> opaque=opaque@entry=0x7ffe9effd954) at libvirt-stream.c:964
> #10 0x56352576232e in cmdVolDownload (ctl=0x7ffe9effda40, cmd= out>) at virsh-volume.c:834
> #11 0x5635257662f1 in vshCommandRun (ctl=0x7ffe9effda40, 
> cmd=0x563525bacf40) at vsh.c:1327
> #12 0x56352572aee2 in main (argc=9, argv=) at virsh.c:929
> 
>  Trying to reproduce yet another one, the command gets stuck even with
>  different offsets.
> 
>  - vol-download --sparse --offset $X --length 1
>/path/to/source.file destination.file
> 
> - This does not respect the length if:
> X > $source_file_size - $last_hole_size
> 
>   The size ends up being $source_file_size - $X

Humble suggestion here: what about turning the simple scenarios above
as proper tests?

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: mkdir memory_backing_dir on startup

[Andrea Bolognani]

On Sat, 2017-05-27 at 12:45 +0200, Michal Privoznik wrote:
> In 48d9e6cdcc and friends we've allowed users to back guest
> memory by a file inside the host. And in order to keep things
> manageable the memory_backing_dir variable was introduced to
> qemu.conf to specify the directory where the files are kept.
> However, libvirt's policy is that directories are created on
> domain startup if they don't exist. We've missed this one.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_driver.c | 12 
>  1 file changed, 12 insertions(+)

ACK and safe for freeze.

Wouldn't mind it if you provided a very brief entry in the
release notes, too ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH go-xml] Add support for Node Device with basic testing.

[Daniel P. Berrange]

On Tue, May 30, 2017 at 10:36:22AM -0400, Vladik Romanovsky wrote:
> Thanks for the review.
> 
> On Tue, May 30, 2017 at 8:46 AM, Daniel P. Berrange  
> wrote:
> >> +type NodeDevice struct {
> >> + Name   string  `xml:"name"`
> >> + Path   string  `xml:"path,omitempty"`
> >> + Parent string  `xml:"parent,omitempty"`
> >> + Driver string  `xml:"driver>name,omitempty"`
> >> + Capability *CapabilityType `xml:"capability"`
> >> +}
> >
> > A single device can have multiple capabilities.
> Do you mean it should be:
>  Capability []CapabilityType `xml:"capability"` ?

Yes exactly.


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 go-xml] 1, add support for disk cache and io, and add test case. 2, add support for controller model, and add test case. 3, extend DomainAddress struct for PCI address and target

[Daniel P. Berrange]

On Wed, May 31, 2017 at 09:26:01AM +0100, Daniel P. Berrange wrote:
> On Wed, May 31, 2017 at 01:32:47PM +0800, ZhenweiPi wrote:
> > ---
> > 
> >  domain.go  | 13 +++--
> >  domain_test.go | 56 
> > +---
> >  2 files changed, 64 insertions(+), 5 deletions(-)
> > 
> > diff --git a/domain.go b/domain.go
> > index 848835a..cbb22e5 100644
> > --- a/domain.go
> > +++ b/domain.go
> > @@ -30,8 +30,10 @@ import (
> >  )
> >  type DomainController struct {
> > -   Type  string `xml:"type,attr"`
> > -   Index string `xml:"index,attr"`
> > +   Typestring `xml:"type,attr"`
> > +   Index   *uint  `xml:"index,attr"`
> > +   Model   string `xml:"model,attr,omitempty"`
> > +   Address *DomainAddress `xml:"address"`
> >  }
> >  type DomainDiskSecret struct {
> > @@ -77,6 +79,8 @@ type DomainDisk struct {
> > Type string`xml:"type,attr"`
> > Device   string`xml:"device,attr"`
> > Snapshot string`xml:"snapshot,attr,omitempty"`
> > +   Cachestring`xml:"cache,attr,omitempty"`
> > +   Io   string`xml:"io,attr,omitempty"`
> > Driver   *DomainDiskDriver `xml:"driver"`
> > Auth *DomainDiskAuth   `xml:"auth"`
> > Source   *DomainDiskSource `xml:"source"`
> > @@ -196,8 +200,13 @@ type DomainAlias struct {
> >  type DomainAddress struct {
> > Type   string `xml:"type,attr"`
> > Controller *uint  `xml:"controller,attr"`
> > +   Domain *uint  `xml:"domain,attr"`
> > Bus*uint  `xml:"bus,attr"`
> > Port   *uint  `xml:"port,attr"`
> > +   Slot   *uint  `xml:"slot,attr"`
> > +   Function   *uint  `xml:"function,attr"`
> > +   Target *uint  `xml:"target,attr"`
> > +   Unit   *uint  `xml:"unit,attr"`
> >  }
> >  type DomainChardev struct {
> > diff --git a/domain_test.go b/domain_test.go
> > index 265cf80..22da947 100644
> > --- a/domain_test.go
> > +++ b/domain_test.go
> > @@ -30,6 +30,16 @@ import (
> > "testing"
> >  )
> > +type PciAddress struct {
> > +   Domain   uint
> > +   Bus  uint
> > +   Slot uint
> > +   Function uint
> > +}
> > +
> > +var uhciIndex uint = 0
> > +var uhciAddr = PciAddress{0, 0, 1, 2}
> 
> This struct and variables are rather pointless - just put the values
> inline in the one place where they are needed

Oh actually I see they are needed - you can't take the address of a
scalar in go.

I'll apply this patch to git.

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 v2] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Martin Kletzander]

On Mon, May 29, 2017 at 02:08:53PM -0400, Daniel Liu wrote:

The option allows someone to run domain-to-native on already existing
domain without the need of supplying their XML.  It is basically
wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
---
tools/virsh-domain.c | 52 
1 file changed, 40 insertions(+), 12 deletions(-)



Just few things I forgot to sent, so they remained in my drafts folder.


diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ccb514ef9..929f9c896 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
static bool
cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
{
-bool ret = true;
+bool ret = false;
const char *format = NULL;
-const char *xmlFile = NULL;
-char *configData;
-char *xmlData;
+const char *domain = NULL;
+const char *xml = NULL;
+char *xmlData = NULL;
+char *configData = NULL;
unsigned int flags = 0;
virshControlPtr priv = ctl->privData;
+virDomainPtr dom = NULL;

if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
-vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
+vshCommandOptStringReq(ctl, cmd, "xml", ) < 0 ||
+vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)


You don't have to do this, you don't use the domain name anywhere, just
the information whether it's present or not.


return false;

-if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
+VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
+
+if (domain) {
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;



And instead of this, you can just do:

if (vshCommandOptBool("domain") &&
   (!(dom = virshCommandOptDomain(ctl, cmd, NULL
   return false;


+}
+
+if (dom) {
+xmlData = virDomainGetXMLDesc(dom, flags);


xmlData can be NULL after this in case of an error.


+} else if (xml) {
+if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
+goto cleanup;
+}
+
+if (!xmlData) {


So this could just be else {} of the last condition and if xmlData is
NULL after calling virDomainGetXMLDesc(), there should be different
error message.

Other things were pointed out already, I guess.


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

Re: [libvirt] [PATCH] udev: Fix build on older platforms

[Erik Skultety]

On Wed, May 31, 2017 at 12:44:37PM +0200, Michal Privoznik wrote:
> On 05/31/2017 12:22 PM, Erik Skultety wrote:
> > Caused by commit @d1eea6c1 due to the missing symbol on older platforms.
> >
> > Signed-off-by: Erik Skultety 
> > ---
> > Despite falling under build-breaker category, I'd like to get a proper 
> > review,
> > since I'm not really familiar with autoconf and there might be a better fix.
> >
> > Erik
> >
> >  configure.ac   | 2 +-
> >  src/node_device/node_device_udev.c | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
>
>
> diff --git a/m4/virt-udev.m4 b/m4/virt-udev.m4
> index 85ca2cb1a..be7dba5d2 100644
> --- a/m4/virt-udev.m4
> +++ b/m4/virt-udev.m4
> @@ -34,6 +34,14 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[
>   if test "$with_udev_logging" = "yes" ; then
>  AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1, [whether libudev logging 
> can be used])
>   fi
> +
> +old_CFLAGS="$CFLAGS"
> +old_LIBS="$LIBS"
> +CFLAGS="$CFLAGS $UDEV_CFLAGS"
> +LIBS="$CFLAGS $UDEV_LIBS"
> +AC_CHECK_FUNCS([udev_monitor_set_receive_buffer_size])
> +CFLAGS="$old_CFLAGS"
> +LIBS="$old_LIBS"
>fi
>  ])
>
>
> This is what I wanted to say, but Daniel beat me to it. ACK if you go this 
> way.

Thanks, help's appreciated, the fix is not pushed.

Erik

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

Re: [libvirt] Availability of libvirt-3.4.0 Release Candidate 2

[Erik Skultety]

On Wed, May 31, 2017 at 12:53:08PM +0200, Andrea Bolognani wrote:
> On Tue, 2017-05-30 at 23:15 +0200, Daniel Veillard wrote:
> >   As scheduled, I have tagged it in git and pushed signed tarball and rpms
> > to the usual place:
> >
> >   ftp://libvirt.org/libvirt/
> >
> >  no difference in my limited testing compared to rc1, looks fine
> > https://ci.centos.org/view/libvirt/ is green except libvirt-master-build
> > being red which looks weird.
>
> The issue was pointed out by Dan in [1], not sure if anyone
> is working on patches. CC'ing the people involved.
>
> If a fix doesn't get merged in time for the release, d1eea6c1

Should be fine now.

Erik

> should be reverted so that libvirt 3.4.0 can be compiled on
> CentOS 6. Unless we decide this is a good time to finally
> leave 6+ years old operating systems behind, that is :)
>
>
> [1] https://www.redhat.com/archives/libvir-list/2017-May/msg01160.html
> --
> Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH v2 1/2] virfdstream: Check for thread error more frequently

[Michal Privoznik]

When the I/O thread quits (e.g. due to an I/O error, lseek()
error, whatever), any subsequent virFDStream API should return
error too. Moreover, when invoking stream event callback, we must
set the VIR_STREAM_EVENT_ERROR flag so that the callback knows
something bad happened.

Signed-off-by: Michal Privoznik 
---
 src/util/virfdstream.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 7ee58be13..cd24757e6 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -312,6 +312,9 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED,
 return;
 }
 
+if (fdst->threadErr)
+events |= VIR_STREAM_EVENT_ERROR;
+
 cb = fdst->cb;
 cbopaque = fdst->opaque;
 ff = fdst->ff;
@@ -787,10 +790,10 @@ static int virFDStreamWrite(virStreamPtr st, const char 
*bytes, size_t nbytes)
 if (fdst->thread) {
 char *buf;
 
-if (fdst->threadQuit) {
+if (fdst->threadQuit || fdst->threadErr) {
 virReportSystemError(EBADF, "%s",
  _("cannot write to stream"));
-return -1;
+goto cleanup;
 }
 
 if (VIR_ALLOC(msg) < 0 ||
@@ -866,7 +869,7 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, 
size_t nbytes)
 virFDStreamMsgPtr msg = NULL;
 
 while (!(msg = fdst->msg)) {
-if (fdst->threadQuit) {
+if (fdst->threadQuit || fdst->threadErr) {
 if (nbytes) {
 virReportSystemError(EBADF, "%s",
  _("stream is not open"));
@@ -959,6 +962,9 @@ virFDStreamSendHole(virStreamPtr st,
 fdst->offset += length;
 }
 
+if (fdst->threadErr)
+goto cleanup;
+
 if (fdst->thread) {
 /* Things are a bit complicated here. If FDStream is in a
  * read mode, then if the message at the queue head is
@@ -1018,6 +1024,9 @@ virFDStreamInData(virStreamPtr st,
 
 virObjectLock(fdst);
 
+if (fdst->threadErr)
+goto cleanup;
+
 if (fdst->thread) {
 virFDStreamMsgPtr msg;
 
-- 
2.13.0

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

Re: [libvirt] [PATCH 0/2] Two simple sparse streams fixes

[Michal Privoznik]

On 05/31/2017 01:03 PM, Martin Kletzander wrote:
> On Tue, May 30, 2017 at 12:44:21PM +0200, Michal Privoznik wrote:
>> I've been experimenting with sparse streams and found a bug. If you
>> try to
>> download a volume which doesn't support sparseness here's what happens:
>>
>> # virsh vol-download --sparse
>> /dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0
>> /mnt/floppy/blah.raw
>>
>> # echo $?
>> 0
>> # ls -lhs /mnt/floppy/bla.raw
>> 0 -rw-r--r-- 1 root root 0 May 30 12:40 /mnt/floppy/bla.raw
>>
>> That's not good. iSCSI doesn't know anything about sparseness so an
>> error is
>> expected here. Fortunately, the fix is fairly simple:
>>
>> # virsh vol-download --sparse
>> /dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0
>> /mnt/floppy/bla.raw
>> error: cannot close volume
>> /dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0
>>
>> error: Unable to seek to data: Invalid argument
>>
> 
> I'm also getting confusing errors when there is no space on the
> destination:
>  error: cannot receive data from volume fedora.img
>  error: An error occurred, but the cause is unknown

Looks like one of the callbacks is not reporting errors.

> 
> But that's not related to the sparse streams (unless it was caused by
> making the iohelper a thread).
> 
> ... few moments later after /me tries just a thing or two ...
> 
> Well, this made me try out few more things and I've found out few
> things.  I'm not sure what's related to your patches and what's not, so
> here's the rundown, and I'll let you decide:
> 
> - vol-download --sparse --offset $source_file_size --length 1
>   /path/to/source.file destination.file
> 
>- Every now and then (not always) it gets stuck waiting for the
>  daemon to receive data (see backtrace below), but the daemon is not
>  waiting for anything, it's just some weird race.  We can try
>  debugging it with wireshark later.  That file ends with a hole.
> 
> Thread 1 (Thread 0x7f1d2b434880 (LWP 28584)):
> #0  0x7f1d2796efbd in poll () at ../sysdeps/unix/syscall-template.S:84
> #1  0x7f1d2a806ee3 in poll (__timeout=5000, __nfds=2,
> __fds=0x7ffe9effd640) at /usr/include/bits/poll2.h:46
> #2  virNetClientIOEventLoop (client=client@entry=0x563525bb06d0,
> thiscall=thiscall@entry=0x563525badc00) at rpc/virnetclient.c:1664
> #3  0x7f1d2a8074d3 in virNetClientIO
> (client=client@entry=0x563525bb06d0, thiscall=0x563525badc00) at
> rpc/virnetclient.c:1957
> #4  0x7f1d2a80780e in virNetClientSendInternal
> (client=client@entry=0x563525bb06d0, msg=msg@entry=0x563525bb03d0,
> expectReply=expectReply@entry=true, nonBlock=nonBlock@entry=false) at
> rpc/virnetclient.c:2132
> #5  0x7f1d2a808dfc in virNetClientSendWithReplyStream
> (client=client@entry=0x563525bb06d0, msg=msg@entry=0x563525bb03d0,
> st=st@entry=0x563525bade10) at rpc/virnetclient.c:2236
> #6  0x7f1d2a80ab2d in virNetClientStreamRecvPacket
> (st=st@entry=0x563525bade10, client=0x563525bb06d0,
> data=data@entry=0x7f1d20686010 "", nbytes=nbytes@entry=262120,
> nonblock=false, flags=32766, flags@entry=1) at rpc/virnetclientstream.c:499
> #7  0x7f1d2a7e0e3e in remoteStreamRecvFlags (st=0x563525badc60,
> data=0x7f1d20686010 "", nbytes=262120, flags=1) at
> remote/remote_driver.c:5664
> #8  0x7f1d2a7c8347 in virStreamRecvFlags
> (stream=stream@entry=0x563525badc60, data=0x7f1d20686010 "",
> nbytes=nbytes@entry=262120, flags=flags@entry=1) at libvirt-stream.c:361
> #9  0x7f1d2a7c9b7f in virStreamSparseRecvAll
> (stream=stream@entry=0x563525badc60, handler=0x563525760196
> , holeHandler=0x56352576020b ,
> opaque=opaque@entry=0x7ffe9effd954) at libvirt-stream.c:964
> #10 0x56352576232e in cmdVolDownload (ctl=0x7ffe9effda40,
> cmd=) at virsh-volume.c:834
> #11 0x5635257662f1 in vshCommandRun (ctl=0x7ffe9effda40,
> cmd=0x563525bacf40) at vsh.c:1327
> #12 0x56352572aee2 in main (argc=9, argv=) at
> virsh.c:929
> 
> Trying to reproduce yet another one, the command gets stuck even with
> different offsets.
> 
> - vol-download --sparse --offset $X --length 1
>   /path/to/source.file destination.file
> 
>- This does not respect the length if:
>X > $source_file_size - $last_hole_size
> 
>  The size ends up being $source_file_size - $X
> 

Okay, I'll look into these. Thanks.

> 
> 
> I'm afraid to try more things, but I can provide more info for these if
> you want.

Don't be! At least somebody is testing the feature. Thanks.

Anyway, I'll send v2 on 1/2.

Michal

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

Re: [libvirt] [PATCH 0/2] Two simple sparse streams fixes

[Martin Kletzander]

On Tue, May 30, 2017 at 12:44:21PM +0200, Michal Privoznik wrote:

I've been experimenting with sparse streams and found a bug. If you try to
download a volume which doesn't support sparseness here's what happens:

# virsh vol-download --sparse 
/dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0 
/mnt/floppy/blah.raw

# echo $?
0
# ls -lhs /mnt/floppy/bla.raw
0 -rw-r--r-- 1 root root 0 May 30 12:40 /mnt/floppy/bla.raw

That's not good. iSCSI doesn't know anything about sparseness so an error is
expected here. Fortunately, the fix is fairly simple:

# virsh vol-download --sparse 
/dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0 
/mnt/floppy/bla.raw
error: cannot close volume 
/dev/disk/by-path/ip-XX.XX.XX.XX:3260-iscsi-iqn.2017-03.com.blah:server-lun-0
error: Unable to seek to data: Invalid argument



I'm also getting confusing errors when there is no space on the
destination:
 error: cannot receive data from volume fedora.img
 error: An error occurred, but the cause is unknown

But that's not related to the sparse streams (unless it was caused by
making the iohelper a thread).

... few moments later after /me tries just a thing or two ...

Well, this made me try out few more things and I've found out few
things.  I'm not sure what's related to your patches and what's not, so
here's the rundown, and I'll let you decide:

- vol-download --sparse --offset $source_file_size --length 1
  /path/to/source.file destination.file

   - Every now and then (not always) it gets stuck waiting for the
 daemon to receive data (see backtrace below), but the daemon is not
 waiting for anything, it's just some weird race.  We can try
 debugging it with wireshark later.  That file ends with a hole.

Thread 1 (Thread 0x7f1d2b434880 (LWP 28584)):
#0  0x7f1d2796efbd in poll () at ../sysdeps/unix/syscall-template.S:84
#1  0x7f1d2a806ee3 in poll (__timeout=5000, __nfds=2, __fds=0x7ffe9effd640) 
at /usr/include/bits/poll2.h:46
#2  virNetClientIOEventLoop (client=client@entry=0x563525bb06d0, 
thiscall=thiscall@entry=0x563525badc00) at rpc/virnetclient.c:1664
#3  0x7f1d2a8074d3 in virNetClientIO (client=client@entry=0x563525bb06d0, 
thiscall=0x563525badc00) at rpc/virnetclient.c:1957
#4  0x7f1d2a80780e in virNetClientSendInternal 
(client=client@entry=0x563525bb06d0, msg=msg@entry=0x563525bb03d0, 
expectReply=expectReply@entry=true, nonBlock=nonBlock@entry=false) at 
rpc/virnetclient.c:2132
#5  0x7f1d2a808dfc in virNetClientSendWithReplyStream 
(client=client@entry=0x563525bb06d0, msg=msg@entry=0x563525bb03d0, 
st=st@entry=0x563525bade10) at rpc/virnetclient.c:2236
#6  0x7f1d2a80ab2d in virNetClientStreamRecvPacket (st=st@entry=0x563525bade10, 
client=0x563525bb06d0, data=data@entry=0x7f1d20686010 "", 
nbytes=nbytes@entry=262120, nonblock=false, flags=32766, flags@entry=1) at 
rpc/virnetclientstream.c:499
#7  0x7f1d2a7e0e3e in remoteStreamRecvFlags (st=0x563525badc60, data=0x7f1d20686010 
"", nbytes=262120, flags=1) at remote/remote_driver.c:5664
#8  0x7f1d2a7c8347 in virStreamRecvFlags (stream=stream@entry=0x563525badc60, 
data=0x7f1d20686010 "", nbytes=nbytes@entry=262120, flags=flags@entry=1) at 
libvirt-stream.c:361
#9  0x7f1d2a7c9b7f in virStreamSparseRecvAll (stream=stream@entry=0x563525badc60, 
handler=0x563525760196 , holeHandler=0x56352576020b 
, opaque=opaque@entry=0x7ffe9effd954) at libvirt-stream.c:964
#10 0x56352576232e in cmdVolDownload (ctl=0x7ffe9effda40, cmd=) at virsh-volume.c:834
#11 0x5635257662f1 in vshCommandRun (ctl=0x7ffe9effda40, 
cmd=0x563525bacf40) at vsh.c:1327
#12 0x56352572aee2 in main (argc=9, argv=) at virsh.c:929

Trying to reproduce yet another one, the command gets stuck even with
different offsets.

- vol-download --sparse --offset $X --length 1
  /path/to/source.file destination.file

   - This does not respect the length if:
   X > $source_file_size - $last_hole_size

 The size ends up being $source_file_size - $X



I'm afraid to try more things, but I can provide more info for these if
you want.

Have a nice day,
Martin


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

Re: [libvirt] Availability of libvirt-3.4.0 Release Candidate 2

[Daniel Veillard]

On Wed, May 31, 2017 at 12:53:08PM +0200, Andrea Bolognani wrote:
> On Tue, 2017-05-30 at 23:15 +0200, Daniel Veillard wrote:
> >   As scheduled, I have tagged it in git and pushed signed tarball and rpms
> > to the usual place:
> > 
> >   ftp://libvirt.org/libvirt/
> > 
> >  no difference in my limited testing compared to rc1, looks fine
> > https://ci.centos.org/view/libvirt/ is green except libvirt-master-build
> > being red which looks weird.
> 
> The issue was pointed out by Dan in [1], not sure if anyone
> is working on patches. CC'ing the people involved.
> 
> If a fix doesn't get merged in time for the release, d1eea6c1
> should be reverted so that libvirt 3.4.0 can be compiled on
> CentOS 6. Unless we decide this is a good time to finally
> leave 6+ years old operating systems behind, that is :)
> 
> 
> [1] https://www.redhat.com/archives/libvir-list/2017-May/msg01160.html

   ok thanks for the heads-up,

  honnestly I would be tempted to wait until we get a patch for this,

Daniel

-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] Availability of libvirt-3.4.0 Release Candidate 2

[Andrea Bolognani]

On Tue, 2017-05-30 at 23:15 +0200, Daniel Veillard wrote:
>   As scheduled, I have tagged it in git and pushed signed tarball and rpms
> to the usual place:
> 
>   ftp://libvirt.org/libvirt/
> 
>  no difference in my limited testing compared to rc1, looks fine
> https://ci.centos.org/view/libvirt/ is green except libvirt-master-build
> being red which looks weird.

The issue was pointed out by Dan in [1], not sure if anyone
is working on patches. CC'ing the people involved.

If a fix doesn't get merged in time for the release, d1eea6c1
should be reverted so that libvirt 3.4.0 can be compiled on
CentOS 6. Unless we decide this is a good time to finally
leave 6+ years old operating systems behind, that is :)


[1] https://www.redhat.com/archives/libvir-list/2017-May/msg01160.html
-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] udev: Fix build on older platforms

[Michal Privoznik]

On 05/31/2017 12:22 PM, Erik Skultety wrote:
> Caused by commit @d1eea6c1 due to the missing symbol on older platforms.
> 
> Signed-off-by: Erik Skultety 
> ---
> Despite falling under build-breaker category, I'd like to get a proper review,
> since I'm not really familiar with autoconf and there might be a better fix.
> 
> Erik
> 
>  configure.ac   | 2 +-
>  src/node_device/node_device_udev.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)


diff --git a/m4/virt-udev.m4 b/m4/virt-udev.m4
index 85ca2cb1a..be7dba5d2 100644
--- a/m4/virt-udev.m4
+++ b/m4/virt-udev.m4
@@ -34,6 +34,14 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[
  if test "$with_udev_logging" = "yes" ; then
 AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1, [whether libudev logging 
can be used])
  fi
+
+old_CFLAGS="$CFLAGS"
+old_LIBS="$LIBS"
+CFLAGS="$CFLAGS $UDEV_CFLAGS"
+LIBS="$CFLAGS $UDEV_LIBS"
+AC_CHECK_FUNCS([udev_monitor_set_receive_buffer_size])
+CFLAGS="$old_CFLAGS"
+LIBS="$old_LIBS"
   fi
 ])


This is what I wanted to say, but Daniel beat me to it. ACK if you go this way.

Michal

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

Re: [libvirt] [PATCH] udev: Fix build on older platforms

[Daniel P. Berrange]

On Wed, May 31, 2017 at 12:22:01PM +0200, Erik Skultety wrote:
> Caused by commit @d1eea6c1 due to the missing symbol on older platforms.
> 
> Signed-off-by: Erik Skultety 
> ---
> Despite falling under build-breaker category, I'd like to get a proper review,
> since I'm not really familiar with autoconf and there might be a better fix.
> 
> Erik
> 
>  configure.ac   | 2 +-
>  src/node_device/node_device_udev.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 246f4e077..b78c8b790 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -322,7 +322,7 @@ AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid 
> getgrnam_r \
>getmntent_r getpwuid_r getrlimit getuid if_indextoname kill mmap \
>newlocale posix_fallocate posix_memalign prlimit regexec \
>sched_getaffinity setgroups setns setrlimit symlink sysctlbyname \
> -  getifaddrs sched_setscheduler unshare])
> +  getifaddrs sched_setscheduler unshare 
> udev_monitor_set_receive_buffer_size])

That is always going to fail - this AC_CHECK_FUNCS_ONCE macro is only
able to detect functions that are part of glibc.


Take a look at m4/virt-dbus.m4 for an example of how to check for a
function in another library, after detecting the library with pkgconfig


>  dnl Availability of various common headers (non-fatal if missing).
>  AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
> diff --git a/src/node_device/node_device_udev.c 
> b/src/node_device/node_device_udev.c
> index a69dc1175..01438ea17 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1764,12 +1764,14 @@ static int nodeStateInitialize(bool privileged,
> 
>  udev_monitor_enable_receiving(priv->udev_monitor);
> 
> +#if HAVE_UDEV_MONITOR_SET_RECEIVE_BUFFER_SIZE
>  /* mimic udevd's behaviour and override the systems rmem_max limit in 
> case
>   * there's a significant number of device 'add' events
>   */
>  if (geteuid() == 0)
>  udev_monitor_set_receive_buffer_size(priv->udev_monitor,
>   128 * 1024 * 1024);
> +#endif
> 
>  /* We register the monitor with the event callback so we are
>   * notified by udev of device changes before we enumerate existing
> --
> 2.13.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

[libvirt] [PATCH] udev: Fix build on older platforms

[Erik Skultety]

Caused by commit @d1eea6c1 due to the missing symbol on older platforms.

Signed-off-by: Erik Skultety 
---
Despite falling under build-breaker category, I'd like to get a proper review,
since I'm not really familiar with autoconf and there might be a better fix.

Erik

 configure.ac   | 2 +-
 src/node_device/node_device_udev.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 246f4e077..b78c8b790 100644
--- a/configure.ac
+++ b/configure.ac
@@ -322,7 +322,7 @@ AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid 
getgrnam_r \
   getmntent_r getpwuid_r getrlimit getuid if_indextoname kill mmap \
   newlocale posix_fallocate posix_memalign prlimit regexec \
   sched_getaffinity setgroups setns setrlimit symlink sysctlbyname \
-  getifaddrs sched_setscheduler unshare])
+  getifaddrs sched_setscheduler unshare udev_monitor_set_receive_buffer_size])

 dnl Availability of various common headers (non-fatal if missing).
 AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index a69dc1175..01438ea17 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1764,12 +1764,14 @@ static int nodeStateInitialize(bool privileged,

 udev_monitor_enable_receiving(priv->udev_monitor);

+#if HAVE_UDEV_MONITOR_SET_RECEIVE_BUFFER_SIZE
 /* mimic udevd's behaviour and override the systems rmem_max limit in case
  * there's a significant number of device 'add' events
  */
 if (geteuid() == 0)
 udev_monitor_set_receive_buffer_size(priv->udev_monitor,
  128 * 1024 * 1024);
+#endif

 /* We register the monitor with the event callback so we are
  * notified by udev of device changes before we enumerate existing
--
2.13.0

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

Re: [libvirt] [PATCH go-xml] 1, add support for disk cache and io, and add test case. 2, add support for controller model, and add test case. 3, extend DomainAddress struct for PCI address and target

[Daniel P. Berrange]

On Wed, May 31, 2017 at 01:32:47PM +0800, ZhenweiPi wrote:
> ---
> 
>  domain.go  | 13 +++--
>  domain_test.go | 56 +---
>  2 files changed, 64 insertions(+), 5 deletions(-)
> 
> diff --git a/domain.go b/domain.go
> index 848835a..cbb22e5 100644
> --- a/domain.go
> +++ b/domain.go
> @@ -30,8 +30,10 @@ import (
>  )
>  type DomainController struct {
> - Type  string `xml:"type,attr"`
> - Index string `xml:"index,attr"`
> + Typestring `xml:"type,attr"`
> + Index   *uint  `xml:"index,attr"`
> + Model   string `xml:"model,attr,omitempty"`
> + Address *DomainAddress `xml:"address"`
>  }
>  type DomainDiskSecret struct {
> @@ -77,6 +79,8 @@ type DomainDisk struct {
>   Type string`xml:"type,attr"`
>   Device   string`xml:"device,attr"`
>   Snapshot string`xml:"snapshot,attr,omitempty"`
> + Cachestring`xml:"cache,attr,omitempty"`
> + Io   string`xml:"io,attr,omitempty"`
>   Driver   *DomainDiskDriver `xml:"driver"`
>   Auth *DomainDiskAuth   `xml:"auth"`
>   Source   *DomainDiskSource `xml:"source"`
> @@ -196,8 +200,13 @@ type DomainAlias struct {
>  type DomainAddress struct {
>   Type   string `xml:"type,attr"`
>   Controller *uint  `xml:"controller,attr"`
> + Domain *uint  `xml:"domain,attr"`
>   Bus*uint  `xml:"bus,attr"`
>   Port   *uint  `xml:"port,attr"`
> + Slot   *uint  `xml:"slot,attr"`
> + Function   *uint  `xml:"function,attr"`
> + Target *uint  `xml:"target,attr"`
> + Unit   *uint  `xml:"unit,attr"`
>  }
>  type DomainChardev struct {
> diff --git a/domain_test.go b/domain_test.go
> index 265cf80..22da947 100644
> --- a/domain_test.go
> +++ b/domain_test.go
> @@ -30,6 +30,16 @@ import (
>   "testing"
>  )
> +type PciAddress struct {
> + Domain   uint
> + Bus  uint
> + Slot uint
> + Function uint
> +}
> +
> +var uhciIndex uint = 0
> +var uhciAddr = PciAddress{0, 0, 1, 2}

This struct and variables are rather pointless - just put the values
inline in the one place where they are needed

> +
>  var domainTestData = []struct {
>   Object   *Domain
>   Expected []string
> @@ -130,10 +140,12 @@ var domainTestData = []struct {
>   },
>   },
>   DomainDisk{
> - Type: "volume",
> + Type:   "volume",
>   Device: "cdrom",
> + Cache:  "none",
> + Io: "native",
>   Source: {
> - Pool: "default",
> + Pool:   "default",
>   Volume: "myvolume",
>   },
>   Target: {
> @@ -174,7 +186,7 @@ var domainTestData = []struct {
>   `  `,
>   `  `,
>   ``,
> - ``,
> + ` io="native">`,
>   `   volume="myvolume">`,
>   `  `,
>   ``,
> @@ -820,6 +832,44 @@ var domainTestData = []struct {
>   ``,
>   },
>   },
> + {
> + Object: {
> + Type: "kvm",
> + Name: "test",
> + Devices: {
> + Controllers: []DomainController{
> + DomainController{
> + Type:  "usb",
> + Index: ,
> + Model: "piix3-uhci",
> + Address: {
> + Type: "pci",
> + Domain:   
> ,
> + Bus:  ,
> + Slot: 
> ,
> + Function: 
> ,
> + },
> + },
> + DomainController{
> + Type:  "usb",
> + Index: nil,
> + Model: "ehci",
> + },
> + 

Re: [libvirt] [PATCH 1/2] virfdstream: Check for thread error more frequently

[Martin Kletzander]

On Tue, May 30, 2017 at 12:44:22PM +0200, Michal Privoznik wrote:

When the I/O thread quits (e.g. due to an I/O error, lseek()
error, whatever), any subsequent virFDStream API should return
error too. Moreover, when invoking stream event callback, we must
set the VIR_STREAM_EVENT_ERROR flag so that the callback knows
something bad happened.

Signed-off-by: Michal Privoznik 
---
src/util/virfdstream.c | 15 +++
1 file changed, 15 insertions(+)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 7ee58be13..ebd0f6cf1 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -312,6 +312,9 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED,
return;
}

+if (fdst->threadErr)
+events |= VIR_STREAM_EVENT_ERROR;
+
cb = fdst->cb;
cbopaque = fdst->opaque;
ff = fdst->ff;
@@ -764,6 +767,9 @@ static int virFDStreamWrite(virStreamPtr st, const char 
*bytes, size_t nbytes)
return -1;
}

+if (fdst->threadErr)
+return -1;
+


It feels like this should be done after locking the object.


if (!fdst) {


Not mentioning it looks like it can be NULL before this check.


virReportError(VIR_ERR_INTERNAL_ERROR,
   "%s", _("stream is not open"));
@@ -844,6 +850,9 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, 
size_t nbytes)
return -1;
}

+if (fdst->threadErr)
+return -1;
+
if (!fdst) {


Same here.

I have no iSCSI to test it with, but it looks OK otherwise.


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

Re: [libvirt] [PATCH] CI: also run tests using updated distro(s)

[Martin Kletzander]

On Tue, May 30, 2017 at 03:52:06PM -0300, Claudio André wrote:

Em 29/05/2017 08:54, Martin Kletzander escreveu:

On Sun, May 28, 2017 at 11:07:41PM -0300, claudioandre...@gmail.com
wrote:

From: Claudio André 


'modprobe -c' fails for some reason.


Ahh! Well, it is possible to run modprobe inside Docker if I enable some
options. I'll
check it.


Is the user inside the container root?  I don't think so.  Some more
info output
could be nice, like 'uname -a', 'id', and so on.


I did it.


One disadvantage for using the script is that it couples al output
together.  I know it's needed for running that inside the container, but
the installation and configuration parts take lot of output that I had
to scroll through.  Could _that_ be separated at least?


You can fold/collapsed the output yourself. That said, I folded the
autogen part, but I'll
keep the make unfolded (the way Travis does). Please, let me know if you
want anything else folded (e.g., hide everything but tests).



That's great.  I was just talking about the fact that I didn't see the
option to keep something folded by default.


And, please:
- access https://travis-ci.org/claudioandre/libvirt/jobs/237625211
- and look at line 485 to see the folding in action



Yeah, this looks great.  I like when it's more compact, I just want to
save some people some time in the future.


Also in your config (not here), you have some differences, for example
compiler: ["gcc"] even though it doesn't take arrays.  It doesn't
matter, I know, it just confused me a bit :D


This was Travis itself. I fixed it, please, confirm that using the link
seen above.


Somehow the -qq didn't work, or you didn't have it in the script in the
build you sent the link to.  That's ione of the things hidden fromt he
Web UI.


Without -qq is much worse. We can:
1. collapse as seen above;
2. redirect stdout to /dev/null (I mean, there is no important
information here).

I prefer (and use) #2.



either -qq collapsed or just put it to /dev/null.  I don't care which
one, if there is a problem with the installation it will be in stderr
anyway.


The silent_if_passes() has some drawbacks.



Sure, I didn't know you can collapse part of the output of one command
and that was the first idea that came to my mind.

So the link shows the output with the current patch?


Claudio



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

Re: [libvirt] [PATCH] qemu: Use iohelper during restore

[Shivaprasad bhat]

Hi All,

Can someone review the patch please ?

Thanks,
Shivaprasad

On Wed, Mar 15, 2017 at 11:22 AM, Shivaprasad bhat <
shivaprasadb...@gmail.com> wrote:

> ping..
>
> On Fri, Jan 27, 2017 at 4:29 PM, Shivaprasad G Bhat <
> sb...@linux.vnet.ibm.com> wrote:
>
>> Commit afe6e58 & c4caab53 made necessary changes to use io-helpers
>> during save and restore. The commit c4caab53 missed to remove the
>> redundant check in qemuDomainSaveImageOpen() because of which
>> virFileWrapperFdNew() is not called if bypass_cache is false.
>>
>> Signed-off-by: Shivaprasad G Bhat 
>> ---
>>  src/qemu/qemu_driver.c |7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 516a851..ac89372 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -6150,9 +6150,11 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
>>  virDomainDefPtr def = NULL;
>>  int oflags = open_write ? O_RDWR : O_RDONLY;
>>  virCapsPtr caps = NULL;
>> +unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;
>>
>>  if (bypass_cache) {
>>  int directFlag = virFileDirectFdFlag();
>> +wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE;
>>  if (directFlag < 0) {
>>  virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> _("bypass cache unsupported by this system"));
>> @@ -6166,9 +6168,8 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
>>
>>  if ((fd = qemuOpenFile(driver, NULL, path, oflags, NULL, NULL)) < 0)
>>  goto error;
>> -if (bypass_cache &&
>> -!(*wrapperFd = virFileWrapperFdNew(, path,
>> -   VIR_FILE_WRAPPER_BYPASS_
>> CACHE)))
>> +if (wrapperFd &&
>> +!(*wrapperFd = virFileWrapperFdNew(, path, wrapperFlags)))
>>  goto error;
>>
>>  if (saferead(fd, , sizeof(header)) != sizeof(header)) {
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list