date:20140506

On 05/05/2014 04:37 PM, Laine Stump wrote:
 This uses the new QEMU_CAPS_HOST_PCI_MULTIDOMAIN capability when
 present, for -devivce pci-assign, -device vfio-pci, and -pcidevice.
 
 While creating tests for this new functionality, I noticed that the
 xmls for two existing tests had erroneously specified an
 until-now-ignored domain=0x0002, so I corrected those two tests, and
 also added two failure tests to be sure that we alert users who
 attempt to use a non-zero domain with a qemu that doesn't support it.
 ---
 Change from V1:
 
 Before pushing, I decided that I really should be checking for non-0
 domain for -pcidevice (an old deprecated method of specifying kvm
 device assignment) as well, rather than simply ignoring it (which is
 what I had been doing by choosing to not support it, as I indicated
 in the commit log for v1 of this patch). And by the time I had put in
 a check for non-0 domain to say we don't support that, it was 90% of
 the way to just supporting it anyway.
 
 So this version adds support for non-0 domain to -pcidevice
 commandlines as well (and also adds an appropriate error message when
 non-0 domain isn't supported by the qemu, which will probably be the
 case 100% of the time anyway - *that* is the important change).

In that case qemuMonitorTextAddPCIHostDevice deserves this error too.

ACK with that added.

Jan



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

[libvirt] [PATCH] docs: update README-hacking

2014-05-06 Thread Chen Hanxiao

We didn't have README-valgrind file now.
So remove related description.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 README-hacking | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README-hacking b/README-hacking
index 80b022c..4e02fd8 100644
--- a/README-hacking
+++ b/README-hacking
@@ -15,7 +15,7 @@ Specific development tools and versions will be checked for 
and listed by
 the bootstrap script.
 
 Valgrind http://valgrind.org/ is also highly recommended, if
-Valgrind supports your architecture. See also README-valgrind.
+Valgrind supports your architecture.
 
 While building from a just-cloned source tree may require installing a
 few prerequisites, later, a plain `git pull  make' should be sufficient.
-- 
1.9.0

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

[libvirt] [PATCH] Fix build wihout macvtap or virtualport

Commit 1b14c44 broke the build on FreeBSD by changing
the signature of a few functions without updating the
corresponding stubs that are used when WITH_MACVTAP
or WITH_VIRTUALPORT is not defined.
---
Pushed under the build-breaker rule.

 src/util/virnetdevmacvlan.c  | 8 
 src/util/virnetdevvportprofile.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 56a5f6b..cb85b74 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1068,13 +1068,13 @@ int virNetDevMacVLanDelete(const char *ifname 
ATTRIBUTE_UNUSED)
 int virNetDevMacVLanCreateWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED,
const virMacAddr *macaddress 
ATTRIBUTE_UNUSED,
const char *linkdev 
ATTRIBUTE_UNUSED,
-   enum virNetDevMacVLanMode mode 
ATTRIBUTE_UNUSED,
+   virNetDevMacVLanMode mode 
ATTRIBUTE_UNUSED,
bool withTap ATTRIBUTE_UNUSED,
int vnet_hdr ATTRIBUTE_UNUSED,
const unsigned char *vmuuid 
ATTRIBUTE_UNUSED,
virNetDevVPortProfilePtr 
virtPortProfile ATTRIBUTE_UNUSED,
char **res_ifname ATTRIBUTE_UNUSED,
-   enum virNetDevVPortProfileOp vmop 
ATTRIBUTE_UNUSED,
+   virNetDevVPortProfileOp vmop 
ATTRIBUTE_UNUSED,
char *stateDir ATTRIBUTE_UNUSED,
virNetDevBandwidthPtr bandwidth 
ATTRIBUTE_UNUSED)
 {
@@ -1100,7 +1100,7 @@ int virNetDevMacVLanRestartWithVPortProfile(const char 
*cr_ifname ATTRIBUTE_UNUS
const char *linkdev 
ATTRIBUTE_UNUSED,
const unsigned char *vmuuid 
ATTRIBUTE_UNUSED,
virNetDevVPortProfilePtr 
virtPortProfile ATTRIBUTE_UNUSED,
-   enum virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED)
+   virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED)
 {
 virReportSystemError(ENOSYS, %s,
  _(Cannot create macvlan devices on this platform));
@@ -1112,7 +1112,7 @@ int virNetDevMacVLanVPortProfileRegisterCallback(const 
char *ifname ATTRIBUTE_UN
  const char *linkdev 
ATTRIBUTE_UNUSED,
  const unsigned char *vmuuid 
ATTRIBUTE_UNUSED,
  virNetDevVPortProfilePtr 
virtPortProfile ATTRIBUTE_UNUSED,
- enum virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED)
+ virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED)
 {
 virReportSystemError(ENOSYS, %s,
  _(Cannot create macvlan devices on this platform));
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 7354b2c..0f43b02 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -1248,7 +1248,7 @@ int virNetDevVPortProfileAssociate(const char 
*macvtap_ifname ATTRIBUTE_UNUSED,
const char *linkdev ATTRIBUTE_UNUSED,
int vf ATTRIBUTE_UNUSED,
const unsigned char *vmuuid ATTRIBUTE_UNUSED,
-   enum virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED,
+   virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED,
bool setlink_only ATTRIBUTE_UNUSED)
 {
 virReportSystemError(ENOSYS, %s,
@@ -1261,7 +1261,7 @@ int virNetDevVPortProfileDisassociate(const char 
*macvtap_ifname ATTRIBUTE_UNUSE
   const virMacAddr *macvtap_macaddr 
ATTRIBUTE_UNUSED,
   const char *linkdev ATTRIBUTE_UNUSED,
   int vf ATTRIBUTE_UNUSED,
-  enum virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED)
+  virNetDevVPortProfileOp vmOp 
ATTRIBUTE_UNUSED)
 {
 virReportSystemError(ENOSYS, %s,
  _(Virtual port profile association not supported on 
this platform));
-- 
1.8.3.2

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

Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

2014-05-06 Thread Igor Mammedov

On Fri, 2 May 2014 11:43:05 -0300
Eduardo Habkost ehabk...@redhat.com wrote:

 On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
  On Wed, 30 Apr 2014 17:29:28 -0300
  Eduardo Habkost ehabk...@redhat.com wrote:
  
   This series allows management code to use object-add on X86CPU 
   subclasses, so it
  Is there any reason why device-add couldn't be used?
 
 It needs to work with -machine none, device_add requires a bus to
 exist, and there is no icc-bus on machine_none.
The thing is that CPUID is a function of machine so using
-machine none will provide only approximately accurate data.
I'm not sure that retrieved possibly not accurate data are useful
for libvirt.

 
 The first thing I considered was making icc-bus user-creatable. Then I
 noticed it wouldn't work because object-add always add objects to
 /objects, not inside the qdev hierarchy (that's where device_add looks
 for the bus).
 
 So, allowing device_add could be possible, but would require changing
 more basic infrastructure: either allowing bus-less devices on
 device_add, or allowing device_add to add devices outside the qdev
 hierarchy, or allowing object-add to create objects outside /objects.
 
 Simply making CPU objects work with object-add was much simpler and less
 intrusive. And it had the interesting side-effect of _not_ doing things
 that are not required for CPU model probing (like creating an actual
 VCPU thread).
 
  
  
   can use it to probe for CPU model information without re-running QEMU. 
   The main
   use case for this is to allow management code to create CPU objects and 
   query
   the feature-words and filtered-features properties on the new 
   objects, to
   find out which features each CPU model needs, and to do the same using the
   host CPU model to check which features can be enabled in a given host.
   
   There's experimental libvirt code to use the new command at:
   https://github.com/ehabkost/libvirt/tree/work/cpu-feature-word-query
   The experimental code just create the CPU objects to query for feature
   information, but doesn't do anything with that data.
   
   Eduardo Habkost (5):
 cpu: Initialize cpu-stopped=true earlier
 cpu: Don't try to pause CPUs if they are already stopped
 pc: Don't crash on apic_accept_pic_intr() if CPU has no apic_state
 target-i386: Make CPU objects user-creatable
 target-i386: Report QOM class name for CPU definitions
   
cpus.c| 13 ++---
exec.c|  1 +
hw/i386/pc.c  |  2 +-
qapi-schema.json  |  6 +-
target-i386/cpu.c |  7 +++
5 files changed, 24 insertions(+), 5 deletions(-)
   
  
 

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

[libvirt] [PATCH] spec: sanlock is x86_64 only on RHEL

Signed-off-by: Jiri Denemark jdene...@redhat.com
---
 libvirt.spec.in | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index aeccf4f..84d5bf3 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -262,16 +262,11 @@
 %endif
 
 # Enable sanlock library for lock management with QEMU
-# Sanlock is available only on i686 x86_64 for RHEL
+# Sanlock is available only on x86_64 for RHEL
 %if 0%{?fedora} = 16
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
-%if 0%{?rhel} == 6
-%ifarch %{ix86} x86_64
-%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
-%endif
-%endif
-%if 0%{?rhel} = 7
+%if 0%{?rhel} = 6
 %ifarch x86_64
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
-- 
1.9.2

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

[libvirt] [PATCH] spec: Don't install nonexistent test_libvirt_lockd.aug

test_libvirt_lockd.aug is only generated when qemu driver is enabled.

Signed-off-by: Jiri Denemark jdene...@redhat.com
---
 libvirt.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 84d5bf3..ae40e0e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1878,7 +1878,9 @@ exit 0
 %{_datadir}/augeas/lenses/virtlockd.aug
 %{_datadir}/augeas/lenses/tests/test_virtlockd.aug
 %{_datadir}/augeas/lenses/libvirt_lockd.aug
+%if %{with_qemu}
 %{_datadir}/augeas/lenses/tests/test_libvirt_lockd.aug
+%endif
 
 %if %{with_polkit}
 %if 0%{?fedora} = 12 || 0%{?rhel} = 6
-- 
1.9.2

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

Re: [libvirt] [PATCH 1/4] esx: parse new URI param 'allow_unsafe'

2014-05-06 Thread Martin Kletzander


On Mon, May 05, 2014 at 01:48:03PM -0600, Eric Blake wrote:

On 05/05/2014 03:47 AM, Martin Kletzander wrote:

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/drvesx.html.in | 14 ++
 src/esx/esx_util.c  | 13 -
 src/esx/esx_util.h  |  2 ++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 0816baf..d941763 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -185,6 +185,20 @@ 
vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com
 override the default port 1080.
 /td
 /tr
+tr
+td
+codeallow_unsafe/code
+/td
+td
+code0/code or code1/code
+/td
+td
+If set to 1, this disables check for supported
+virtualHW.version, so connections to newer versions
+than supported is possible.  The default value is 0.


connections is plural, so s/is possible/are possible/

On the surface this (and the rest of the series) makes sense - it allows
older libvirt to be used with a newer ESX version which may or may not
work, until such time as newer libvirt has been tested to explicitly
work with that ESX.  But I'd like an opinion from someone that actually
uses esx:// URIs before checking this in.



Since the vmx parsing was moved to vmx/vmx.c (so basically since it
was introduced), we were blindly (or at least in some cases) adding
support for newer virtualHW_versions (version 8 in commit 5759a5cc,
9 in e0eb672e and finally 10 now in 5cc3cce5).  I tried getting
someone to test the version I added, but since there's so few of us
working with ESX (I had to ask Matthias if he can have a look at the
code), we can only assume that it will work.  I even remember someone
filing a bug that the connection works and everything, but there's a
warning.  My thinking was that we have two options here:

1) continue messing the code with blindly adding versions of
   virtualHW or

2) conditionally allow anything the user wants, with the condition
   showing that we don't support it.

My reasoning behind choosing version 2 was that the development around
VMX-related backends is not that active as with other ones.  So if
anything doesn't work because of HW version it is somehow visible that
the user is not using supported versions of vmx descriptors (or how
to call the .vmx file).  I designed the patch so there is no
functional nor API change with current usage, it merely allows to
check whether our code works with newer versions without touching the
code (currently, when someone wants to try that, he needs to add the
version to supported ones, obviously).  And it still leaves the list
of supported versions in place and whoever wants some version to be
supported can add it to the list.

I know it is impossible to force people fix everything for versions
they added, but it could be at least a bit visible that we currently
don't have the capacity for that.

Martin


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

Re: [libvirt] [PATCH v3 1/3] qemu: extract PCI handling structs

On Mon, May 05, 2014 at 05:00:34PM +0300, Laine Stump wrote:
 On 05/05/2014 03:07 PM, Ján Tomko wrote:
  On 05/03/2014 06:31 PM, Roman Bogorodskiy wrote:
  Extract PCI handling related structs that could be shared
  with other drivers.
 
  List of structs moved to virpci.h and new names:
 
   qemuDomainPCIAddressBus - virDomainPCIAddressBus
   qemuDomainPCIAddressBusPtr - virDomainPCIAddressBusPtr
   _qemuDomainPCIAddressSet - virDomainPCIAddressSet
   qemuDomainPCIAddressSetPtr - virDomainPCIAddressSetPtr
   qemuDomainPCIConnectFlags - virDomainPCIConnectFlags
  I would drop the 'Domain', to make the prefix match the file.
 
 I was thinking about that and came to a different opinion. The functions
 that are currently in virpci.c are dealing with manipulating and
 reporting about PCI devices on the *host* (reading and writing sysfs
 files to attach and detach drivers, determining the list of virtual
 functions for an SRIOV physical function, etc), while these functions
 that Roman is moving are only concerned with managing the allocation of
 PCI addresses to devices in a domain.
 
 Because of that, I think it's reasonable (a good idea really) to keep
 Domain in the function names.
 
 Beyond that, I was going to say that I think these functions belong in
 their own file, *not* virpci.c (and maybe we even want to rename
 virpci.c to virhostpci.c or something). I think it's *essential* that
 the two sets of functions are separated from each other, since what is
 in virpci.c is Linux-specific, but the virDomainPCI... functions should
 be host-agnostic.

Agreed, I think this should go into a src/conf/domain_addr.{c,h}
file since it has a dep on other things in src/conf, which is
not allowed for files in src/util.



Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [libvirt-python PATCH] Bump version to 1.2.5 for new dev cycle

On Mon, May 05, 2014 at 03:57:24PM +0200, Martin Kletzander wrote:
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 
 Notes:
 I'm not sure whether this can be pushed automatically as there is no
 release, just a tag in the repo.  However, libvirt-python fails to
 install without this commit on my machine.

ACK, we should always bump python version at same time as libvirt
version


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH] LXC: don't doubly link /dev/console

On Mon, May 05, 2014 at 11:14:18AM -0400, Dwight Engen wrote:
 When a console is configured, /dev/console and /dev/tty1 are created as
 symlinks to the same underlying pts. This causes problems since a
 separate getty will be spawned for /dev/console and /dev/tty1, but they
 are each controlling the same device.

That is simply a bug in the OS setup IMHO.

FYI systemd specified that the 'container_ttys' env variable should
be set to indicate which devices a gettty should be spawned on and
libvirt now sets that. We intentionally exclude tty1 from the
container_ttys env var so we don't get the double-getty with any
systemd enabled OS.

IMHO even non-systemd OS could read that env var to decide where
to spawn getttys

See also this doc:

  http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface/

 This patch makes it so that /dev/console is the sole symlink to the first
 console, /dev/tty1 to the second, /dev/tty2 to the third and so on.

This is a backwards-incompatible change that is likely to break
existing deployments I'm afraid, so we can't do that IMHO.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] Request for Wiki editing access

On Mon, May 05, 2014 at 06:01:41PM -0500, James Thornsberry wrote:
 To whom it may concern,
 
 I’m just beginning to use the libvirt suite of tools and, in going through 
 your wiki, I’ve found a couple of minor things (mostly typos) that I would 
 immediately correct.  
 
 Also, if I get my project up, I may be able to provide some less-trivial 
 examples of network setup xml as well.
 
 Please feel free to NOT forward this email after moderation as it will likely 
 be of little interest to the group at large. ;-)

FYI, I'm taking care of this request offlist.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [libvirt PATCH] udev: consider the device a CDROM when ID_CDROM=1

2014-05-06 Thread Giuseppe Scrivano

Daniel P. Berrange berra...@redhat.com writes:

 On Wed, Apr 23, 2014 at 12:42:01PM +0200, Giuseppe Scrivano wrote:
 Some CDROM devices are reported by udev to have an ID_TYPE=generic
 thus it is necessary to check if ID_CDROM is present.
 
 As a side effect, treating ID_TYPE=generic as a missing ID_TYPE will
 enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the
 udevKludgeStorageType heuristic.
 
 Signed-off-by: Giuseppe Scrivano gscri...@redhat.com
 ---
  src/node_device/node_device_udev.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 ACK

Thanks.

Can somebody please push the patch as I've no write access to the repo?

Giuseppe

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

Re: [libvirt] [PATCHv2] qemu: specify domain in host-side PCI addresses when needed/supported

2014-05-06 Thread Laine Stump

On 05/06/2014 10:01 AM, Ján Tomko wrote:
 On 05/05/2014 04:37 PM, Laine Stump wrote:
 This uses the new QEMU_CAPS_HOST_PCI_MULTIDOMAIN capability when
 present, for -devivce pci-assign, -device vfio-pci, and -pcidevice.

 While creating tests for this new functionality, I noticed that the
 xmls for two existing tests had erroneously specified an
 until-now-ignored domain=0x0002, so I corrected those two tests, and
 also added two failure tests to be sure that we alert users who
 attempt to use a non-zero domain with a qemu that doesn't support it.
 ---
 Change from V1:

 Before pushing, I decided that I really should be checking for non-0
 domain for -pcidevice (an old deprecated method of specifying kvm
 device assignment) as well, rather than simply ignoring it (which is
 what I had been doing by choosing to not support it, as I indicated
 in the commit log for v1 of this patch). And by the time I had put in
 a check for non-0 domain to say we don't support that, it was 90% of
 the way to just supporting it anyway.

 So this version adds support for non-0 domain to -pcidevice
 commandlines as well (and also adds an appropriate error message when
 non-0 domain isn't supported by the qemu, which will probably be the
 case 100% of the time anyway - *that* is the important change).
 In that case qemuMonitorTextAddPCIHostDevice deserves this error too.

 ACK with that added.

Okay. Done, and pushed both patches.

Thanks!

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

Re: [libvirt] [PATCH] spec: sanlock is x86_64 only on RHEL

On 05/06/2014 01:40 AM, Jiri Denemark wrote:
 Signed-off-by: Jiri Denemark jdene...@redhat.com
 ---
  libvirt.spec.in | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)
 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index aeccf4f..84d5bf3 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -262,16 +262,11 @@
  %endif
  
  # Enable sanlock library for lock management with QEMU
 -# Sanlock is available only on i686 x86_64 for RHEL
 +# Sanlock is available only on x86_64 for RHEL
  %if 0%{?fedora} = 16
  %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
  %endif
 -%if 0%{?rhel} == 6
 -%ifarch %{ix86} x86_64
 -%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 -%endif
 -%endif
 -%if 0%{?rhel} = 7
 +%if 0%{?rhel} = 6
  %ifarch x86_64
  %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
  %endif

The only real change you are making here is that you are crippling
sanlock for ix86 on RHEL 6 (RHEL 7 is unchanged).  Meanwhile, I compared
to the spec file that shipped with RHEL 6.5:

# Sanlock is available only on x86_64 for RHEL-6 on all arches after that
%if 0%{?fedora} = 16
%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
%endif
%if 0%{?rhel} == 6
%ifarch x86_64
%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
%endif
%endif
%if 0%{?rhel} = 7
%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
%endif

which enabled sanlock only on x86_64 for RHEL 6, and unconditionally
enables it for RHEL 7.  Then when you look at the current RHEL 7 beta
product line, it is not built for ix86, only x86_64.  So your
formulation is shorter and appears to accomplish the same limitations,
as well as having the benefit of matching what downstream already changed.

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] spec: Don't install nonexistent test_libvirt_lockd.aug

On 05/06/2014 01:41 AM, Jiri Denemark wrote:
 test_libvirt_lockd.aug is only generated when qemu driver is enabled.
 
 Signed-off-by: Jiri Denemark jdene...@redhat.com
 ---
  libvirt.spec.in | 2 ++
  1 file changed, 2 insertions(+)

ACK

 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index 84d5bf3..ae40e0e 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1878,7 +1878,9 @@ exit 0
  %{_datadir}/augeas/lenses/virtlockd.aug
  %{_datadir}/augeas/lenses/tests/test_virtlockd.aug
  %{_datadir}/augeas/lenses/libvirt_lockd.aug
 +%if %{with_qemu}
  %{_datadir}/augeas/lenses/tests/test_libvirt_lockd.aug
 +%endif
  
  %if %{with_polkit}
  %if 0%{?fedora} = 12 || 0%{?rhel} = 6
 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] spec: Don't install nonexistent test_libvirt_lockd.aug

On Tue, May 06, 2014 at 05:59:00 -0600, Eric Blake wrote:
 On 05/06/2014 01:41 AM, Jiri Denemark wrote:
  test_libvirt_lockd.aug is only generated when qemu driver is enabled.
  
  Signed-off-by: Jiri Denemark jdene...@redhat.com
  ---
   libvirt.spec.in | 2 ++
   1 file changed, 2 insertions(+)
 
 ACK

Pushed, thanks.

Jirka

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

Re: [libvirt] [PATCH] spec: sanlock is x86_64 only on RHEL

On Tue, May 06, 2014 at 05:58:32 -0600, Eric Blake wrote:
 On 05/06/2014 01:40 AM, Jiri Denemark wrote:
  Signed-off-by: Jiri Denemark jdene...@redhat.com
  ---
   libvirt.spec.in | 9 ++---
   1 file changed, 2 insertions(+), 7 deletions(-)
  
  diff --git a/libvirt.spec.in b/libvirt.spec.in
  index aeccf4f..84d5bf3 100644
  --- a/libvirt.spec.in
  +++ b/libvirt.spec.in
  @@ -262,16 +262,11 @@
   %endif
   
   # Enable sanlock library for lock management with QEMU
  -# Sanlock is available only on i686 x86_64 for RHEL
  +# Sanlock is available only on x86_64 for RHEL
   %if 0%{?fedora} = 16
   %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
   %endif
  -%if 0%{?rhel} == 6
  -%ifarch %{ix86} x86_64
  -%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
  -%endif
  -%endif
  -%if 0%{?rhel} = 7
  +%if 0%{?rhel} = 6
   %ifarch x86_64
   %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
   %endif
 
 The only real change you are making here is that you are crippling
 sanlock for ix86 on RHEL 6 (RHEL 7 is unchanged).  Meanwhile, I compared
 to the spec file that shipped with RHEL 6.5:
 
 # Sanlock is available only on x86_64 for RHEL-6 on all arches after that
 %if 0%{?fedora} = 16
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
 %if 0%{?rhel} == 6
 %ifarch x86_64
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
 %endif
 %if 0%{?rhel} = 7
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
 
 which enabled sanlock only on x86_64 for RHEL 6, and unconditionally
 enables it for RHEL 7.  Then when you look at the current RHEL 7 beta
 product line, it is not built for ix86, only x86_64.  So your
 formulation is shorter and appears to accomplish the same limitations,
 as well as having the benefit of matching what downstream already changed.
 
 ACK.

Pushed, thanks.

Jirka

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

Re: [libvirt] [PATCH 3/5] conf: changing all the enum network filter structures into typedef's in src/conf/ directory.

2014-05-06 Thread Julio Faracco

It's strange.

Sure, Eric. I will check why it failed to compile.
No threre is no problem with git rebase -i. But If I need some help
I will ask you in IRC.

Thanks!

--
Julio Cesar Faracco

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

[libvirt] Virsh Attach Volume Fails

2014-05-06 Thread Vilobh Meshram

Hi,

When trying to attach a created volume to an instance using the virsh utility I 
am running into this error :-


[] $ sudo virsh attach-device instance-0015 disk.xml

error: Failed to attach device from disk.xml

error: internal error unable to execute QEMU command '__com.redhat_drive_add': 
Device 'drive-virtio-disk21' could not be initialized


For experimental purpose I have libvirt 0.10.2 and QEMU 0.12.1. The volume to 
be attached is on a remote storage.


Any idea what I might be missing ?


Thanks,

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

[libvirt] [RFC PATCH] Add support for invtsc timer

Not yet merged in upstream QEMU:
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html

Add support for invariant TSC timer running at constant rate in
all ACPI P-, C- and T-states.

It can be enabled by specifying:
clock
  timer name='invtsc' present='yes'/
/clock
in the domain XML.

Migration and saving the domain does not work with this timer.

The support for this timer is indicated by bit 8 of EDX after calling
CPUID with 0x8007. It does not show up in /proc/cpuinfo [1]
and since we're calling qemu without 'enforce', it doesn't error
out if the host doesn't support this.

Alternatively, we could expose it in libvirt as a cpu flag:
cpu mode='custom' match='exact'
  model fallback='forbid'qemu64/model
  feature policy='require' name='invtsc'/
/cpu
or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is
requested?

[1]:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/powerflags.c?id=30321c7b#n18
---
 docs/formatdomain.html.in  |  9 ++--
 docs/schemas/domaincommon.rng  |  1 +
 src/conf/domain_conf.c |  6 +++--
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_command.c|  6 +
 src/qemu/qemu_migration.c  | 14 
 .../qemuxml2argv-clock-timer-inv-tsc.args  |  5 +
 .../qemuxml2argv-clock-timer-inv-tsc.xml   | 26 ++
 tests/qemuxml2argvtest.c   |  1 +
 tests/qemuxml2xmltest.c|  1 +
 10 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4f19473..1d3fd93 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1368,13 +1368,18 @@
 being modified, and can be one of
 platform (currently unsupported),
 hpet (libxl, xen, qemu), kvmclock (qemu),
-pit (qemu), rtc (qemu), tsc (libxl) or hypervclock
-(qemu - span class=sincesince 1.2.2/span).
+pit (qemu), rtc (qemu), tsc (libxl), hypervclock
+(qemu - span class=sincesince 1.2.2/span) or
+invtsc (qemu - span class=sincesince 1.2.5/span).
 
 The codehypervclock/code timer adds support for the
 reference time counter and the reference page for iTSC
 feature for guests running the Microsoft Windows
 operating system.
+
+The codeinvtsc/code timer adds support for the invariant
+TSC. It runs at a constant rate in all ACPI P- C- and T-states.
+A guest with this timer enabled cannot be migrated or saved.
   /dd
   dtcodetrack/code/dt
   dd
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4249ed5..5154826 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -918,6 +918,7 @@
 choice
   valuekvmclock/value
   valuehypervclock/value
+  valueinvtsc/value
 /choice
   /attribute
 /group
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c3bdad..893d904 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -706,7 +706,8 @@ VIR_ENUM_IMPL(virDomainTimerName, 
VIR_DOMAIN_TIMER_NAME_LAST,
   hpet,
   tsc,
   kvmclock,
-  hypervclock);
+  hypervclock,
+  invtsc);
 
 VIR_ENUM_IMPL(virDomainTimerTrack, VIR_DOMAIN_TIMER_TRACK_LAST,
   boot,
@@ -2931,7 +2932,8 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 virDomainTimerDefPtr timer = def-clock.timers[i];
 
 if (timer-name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK ||
-timer-name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK) {
+timer-name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK ||
+timer-name == VIR_DOMAIN_TIMER_NAME_INVTSC) {
 if (timer-tickpolicy != -1) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(timer %s doesn't support setting of 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a92f0f3..53c02e6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1635,6 +1635,7 @@ enum virDomainTimerNameType {
 VIR_DOMAIN_TIMER_NAME_TSC,
 VIR_DOMAIN_TIMER_NAME_KVMCLOCK,
 VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK,
+VIR_DOMAIN_TIMER_NAME_INVTSC,
 
 VIR_DOMAIN_TIMER_NAME_LAST
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c1e17d..2994427 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6736,6 +6736,11 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver,

[libvirt] [PATCH RFC 12/22] test: storage: Initialize storage source to correct type

Stat the path of the storage file being tested to set the correct type
into the virStorageSource. This will avoid breaking the test suite when
inquiring metadata of directory paths in the next patches.
---
 tests/virstoragetest.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index fb96c71..746bf63 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -98,6 +98,7 @@ testStorageFileGetMetadata(const char *path,
uid_t uid, gid_t gid,
bool allow_probe)
 {
+struct stat st;
 virStorageSourcePtr ret = NULL;

 if (VIR_ALLOC(ret)  0)
@@ -106,6 +107,15 @@ testStorageFileGetMetadata(const char *path,
 ret-type = VIR_STORAGE_TYPE_FILE;
 ret-format = format;

+if (stat(path, st) == 0) {
+if (S_ISDIR(st.st_mode)) {
+ret-type = VIR_STORAGE_TYPE_DIR;
+ret-format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+ret-type = VIR_STORAGE_TYPE_BLOCK;
+}
+}
+
 if (VIR_STRDUP(ret-relPath, path)  0)
 goto error;

-- 
1.9.2

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

[libvirt] [PATCH RFC 11/22] storage: Determine the local storage type right away

When walking the backing chain we previously set the storage type to
_FILE and let the virStorageFileGetMetadataFromFDInternal update it to
the correct type later on.

This patch moves the actual storage type determination to the place
where we parse the backing store name so that the code can later be
switched to use virStorageFileReadHeader() directly.
---
 src/storage/storage_driver.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 25772e8..163b402 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3084,6 +3084,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 {
 int fd;
 int ret = -1;
+struct stat st;
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

@@ -3146,6 +3147,16 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 ret = 0;
 goto cleanup;
 }
+
+/* update the type for local storage */
+if (stat(backingStore-path, st) == 0) {
+if (S_ISDIR(st.st_mode)) {
+backingStore-type = VIR_STORAGE_TYPE_DIR;
+backingStore-format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+backingStore-type = VIR_STORAGE_TYPE_BLOCK;
+}
+}
 } else {
 /* TODO: To satisfy the test case, copy the network URI as path. This
  * will be removed later. */
-- 
1.9.2

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

[libvirt] [RESEND PATCH] Add support for timestamping QEMU logs

QEMU commit 5e2ac51 added a boolean '-msg timestamp=[on|off]'
option, which can enable timestamps on errors:
$ qemu-system-x86_64 -msg timestamp=on zghhdorf
2014-04-09T13:25:46.779484Z qemu-system-x86_64: -msg timestamp=on: could
not open disk image zghhdorf: Could not open 'zghhdorf': No such file or
directory

Enable this timestamp if the QEMU binary supports it.

Add a 'log_timestamp' option to qemu.conf for disabling this behavior.
---
Rebased to latest master, this time with the test files added.

previous posting of the patch:
https://www.redhat.com/archives/libvir-list/2014-April/msg00422.html

 src/qemu/libvirtd_qemu.aug |  3 ++
 src/qemu/qemu.conf |  8 ++
 src/qemu/qemu_capabilities.c   |  5 
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c|  4 +++
 src/qemu/qemu_conf.c   |  4 +++
 src/qemu/qemu_conf.h   |  2 ++
 src/qemu/test_libvirtd_qemu.aug.in |  1 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |  1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |  1 +
 .../qemuxml2argv-minimal-msg-timestamp.args|  6 
 .../qemuxml2argv-minimal-msg-timestamp.xml | 32 ++
 tests/qemuxml2argvtest.c   |  1 +
 13 files changed, 69 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-minimal-msg-timestamp.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-minimal-msg-timestamp.xml

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index a9ff421..e985d22 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -85,6 +85,8 @@ module Libvirtd_qemu =
  | int_entry migration_port_min
  | int_entry migration_port_max
 
+   let log_entry = bool_entry log_timestamp
+
(* Each entry in the config is one of the following ... *)
let entry = vnc_entry
  | spice_entry
@@ -96,6 +98,7 @@ module Libvirtd_qemu =
  | device_entry
  | rpc_entry
  | network_entry
+ | log_entry
 
let comment = [ label #comment . del /#[ \t]*/ #  .  store /([^ 
\t\n][^\n]*)?/ . del /\n/ \n ]
let empty = [ label #empty . eol ]
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index f0e802f..42f812d 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -465,3 +465,11 @@
 #
 #migration_port_min = 49152
 #migration_port_max = 49215
+
+
+
+# Timestamp QEMU's log messages (if QEMU supports it)
+#
+# Defaults to 1.
+#
+#log_timestamp = 0
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8343258..0d08e20 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -255,6 +255,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
   usb-kbd, /* 165 */
   host-pci-multidomain,
+  msg-timestamp,
 );
 
 
@@ -1146,6 +1147,9 @@ virQEMUCapsComputeCmdFlags(const char *help,
 if (strstr(help, -machine))
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT);
 
+if (strstr(help, -msg timestamp))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP);
+
 /*
  * Handling of -incoming arg with varying features
  *  -incoming tcp(kvm = 79, qemu = 0.10.0)
@@ -2378,6 +2382,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { boot-opts, strict, QEMU_CAPS_BOOT_STRICT },
 { boot-opts, reboot-timeout, QEMU_CAPS_REBOOT_TIMEOUT },
 { spice, disable-agent-file-xfer, QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
+{ msg, timestamp, QEMU_CAPS_MSG_TIMESTAMP },
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 26400d2..d755caa 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -205,6 +205,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_CHARDEV_SPICEPORT  = 164, /* -chardev spiceport */
 QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
 QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain  0 in host pci 
address */
+QEMU_CAPS_MSG_TIMESTAMP  = 167, /* -msg timestamp */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 29ae8e4..dd8e40a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9742,6 +9742,10 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandSetMaxMemLock(cmd, memKB * 1024);
 }
 
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP) 
+cfg-logTimestamp)
+virCommandAddArgList(cmd, -msg, timestamp=on, NULL);
+
 virObjectUnref(cfg);
 return cmd;
 
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 198ee2f..e487f5e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -255,6 +255,8 @@

[libvirt] [PATCH RFC 22/22] [DO NOT APPLY UPSTREAM] storage: gluster: mock up path canonicalization

Add code to fake gluster storage path canonicalization while I'm working
on the code.
---
 src/storage/storage_backend_gluster.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index 1a844c9..411851b 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -533,6 +533,7 @@ typedef virStorageFileBackendGlusterPriv 
*virStorageFileBackendGlusterPrivPtr;

 struct _virStorageFileBackendGlusterPriv {
 glfs_t *vol;
+char *uid;
 };


@@ -547,6 +548,7 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)

 if (priv-vol)
 glfs_fini(priv-vol);
+VIR_FREE(priv-uid);

 VIR_FREE(priv);
 src-drv-priv = NULL;
@@ -606,6 +608,10 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
 goto error;
 }

+if (virAsprintf(priv-uid, gluster://%s:%d/%s/%s, hostname, port,
+src-volume, src-path)  0)
+goto error;
+
 src-drv-priv = priv;

 return 0;
@@ -713,6 +719,15 @@ virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
 }


+static const char *
+virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+
+return priv-uid;
+}
+
+
 virStorageFileBackend virStorageFileBackendGluster = {
 .type = VIR_STORAGE_TYPE_NETWORK,
 .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER,
@@ -724,4 +739,8 @@ virStorageFileBackend virStorageFileBackendGluster = {
 .storageFileStat = virStorageFileBackendGlusterStat,
 .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
 .storageFileAccess = virStorageFileBackendGlusterAccess,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendGlusterGetUniqueIdentifier,
+
+
 };
-- 
1.9.2

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

[libvirt] [PATCH RFC 01/22] qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage

Refactor the function to accept a virStorageSourcePtr instead of just
the path, add a check to run it only on local storage and fix callers
(possibly by using a newly introduced wrapper that wraps a path in the
 virStorageSource struct for legacy code)
---
 src/qemu/qemu_driver.c | 77 --
 1 file changed, 49 insertions(+), 28 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fca1a91..ac0de1e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12013,22 +12013,26 @@ static int
 qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainDiskDefPtr disk,
-  const char *file,
+  virStorageSourcePtr elem,
   qemuDomainDiskChainMode mode)
 {
 /* The easiest way to label a single file with the same
  * permissions it would have as if part of the disk chain is to
  * temporarily modify the disk in place.  */
-char *origsrc = disk-src.path;
-int origformat = disk-src.format;
-virStorageSourcePtr origchain = disk-src.backingStore;
+virStorageSource origdisk;
 bool origreadonly = disk-readonly;
+virQEMUDriverConfigPtr cfg = NULL;
 int ret = -1;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

-disk-src.path = (char *) file; /* casting away const is safe here */
-disk-src.format = VIR_STORAGE_FILE_RAW;
-disk-src.backingStore = NULL;
+/* XXX Labelling of non-local files isn't currently supported */
+if (virStorageSourceGetActualType(disk-src) == VIR_STORAGE_TYPE_NETWORK)
+return 0;
+
+cfg = virQEMUDriverGetConfig(driver);
+
+/* XXX This will be easier when disk-src will be a pointer */
+memcpy(origdisk, disk-src, sizeof(origdisk));
+memcpy(disk-src, elem, sizeof(*elem));
 disk-readonly = mode == VIR_DISK_CHAIN_READ_ONLY;

 if (mode == VIR_DISK_CHAIN_NO_ACCESS) {
@@ -12051,9 +12055,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr 
driver,
 ret = 0;

  cleanup:
-disk-src.path = origsrc;
-disk-src.format = origformat;
-disk-src.backingStore = origchain;
+memcpy(disk-src, origdisk, sizeof(origdisk));
 disk-readonly = origreadonly;
 virObjectUnref(cfg);
 return ret;
@@ -12061,6 +12063,25 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr 
driver,


 static int
+qemuDomainPrepareDiskChainElementPath(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virDomainDiskDefPtr disk,
+  const char *file,
+  qemuDomainDiskChainMode mode)
+{
+virStorageSource src;
+
+memset(src, 0, sizeof(src));
+
+src.type = VIR_STORAGE_TYPE_FILE;
+src.format = VIR_STORAGE_FILE_RAW;
+src.path = (char *) file; /* casting away const is safe here */
+
+return qemuDomainPrepareDiskChainElement(driver, vm, disk, src, mode);
+}
+
+
+static int
 qemuDomainSnapshotFSFreeze(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm-privateData;
@@ -12765,9 +12786,9 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 VIR_FORCE_CLOSE(fd);
 }

-if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+if (qemuDomainPrepareDiskChainElement(driver, vm, disk, snap-src,
   VIR_DISK_CHAIN_READ_WRITE)  0) {
-qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+qemuDomainPrepareDiskChainElement(driver, vm, disk, snap-src,
   VIR_DISK_CHAIN_NO_ACCESS);
 goto cleanup;
 }
@@ -12898,7 +12919,7 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr 
driver,
 (persistDisk  VIR_STRDUP(persistSource, source)  0))
 goto cleanup;

-qemuDomainPrepareDiskChainElement(driver, vm, disk, disk-src.path,
+qemuDomainPrepareDiskChainElement(driver, vm, disk, disk-src,
   VIR_DISK_CHAIN_NO_ACCESS);
 if (need_unlink 
 virStorageFileStat(disk-src, st) == 0  S_ISREG(st.st_mode) 
@@ -15208,10 +15229,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
 if (VIR_STRDUP(mirror, dest)  0)
 goto endjob;

-if (qemuDomainPrepareDiskChainElement(driver, vm, disk, dest,
-  VIR_DISK_CHAIN_READ_WRITE)  0) {
-qemuDomainPrepareDiskChainElement(driver, vm, disk, dest,
-  VIR_DISK_CHAIN_NO_ACCESS);
+if (qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest,
+  VIR_DISK_CHAIN_READ_WRITE)  0) {
+qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest,
+

Re: [libvirt] [PATCH] util: changing all the enum structures into typedef's in src/util/ directory.

2014-05-06 Thread Julio Faracco

Yes, I choose to do all this mechanical changes first and after I would
change some particular modifications according the previous
those mechanical changes.

First, I would like to change enum's to typedef's.

Thanks for your suggestions, Eric. It was so helpful.
Specially for the short commit message.
I forgot to check it when I sent my patch.

--
Julio Cesar Faracco

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

[libvirt] [PATCH RFC 04/22] conf: Fix domain disk path iterator to work with networked storage

Skip networked storage but continue iteration through backing chain to
iterate through all the local paths in the backing chain.
---
 src/conf/domain_conf.c | 37 -
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3cb6693..25758d9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18686,34 +18686,37 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
 int ret = -1;
 size_t depth = 0;
 virStorageSourcePtr tmp;
-const char *path = virDomainDiskGetSource(disk);
-int type = virDomainDiskGetType(disk);
+char *brokenRaw = NULL;

-if (!path || type == VIR_STORAGE_TYPE_NETWORK ||
-(type == VIR_STORAGE_TYPE_VOLUME 
- disk-src.srcpool 
- disk-src.srcpool-mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT))
-return 0;
-
-if (iter(disk, path, 0, opaque)  0)
-goto cleanup;
+if (!ignoreOpenFailure) {
+if (virStorageFileChainGetBroken(disk-src, brokenRaw)  0)
+goto cleanup;

-tmp = disk-src.backingStore;
-while (tmp  virStorageIsFile(tmp-path)) {
-if (!ignoreOpenFailure  tmp-backingStoreRaw  !tmp-backingStore) {
+if (brokenRaw) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unable to visit backing chain file %s),
-   tmp-backingStoreRaw);
+   brokenRaw);
 goto cleanup;
 }
-if (iter(disk, tmp-path, ++depth, opaque)  0)
-goto cleanup;
-tmp = tmp-backingStore;
+}
+
+for (tmp = disk-src; tmp; tmp = tmp-backingStore) {
+int actualType = virStorageSourceGetActualType(tmp);
+/* execute the callback only for local storage */
+if (actualType != VIR_STORAGE_TYPE_NETWORK 
+actualType != VIR_STORAGE_TYPE_VOLUME 
+tmp-path) {
+if (iter(disk, tmp-path, depth, opaque)  0)
+goto cleanup;
+}
+
+depth++;
 }

 ret = 0;

  cleanup:
+VIR_FREE(brokenRaw);
 return ret;
 }

-- 
1.9.2

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

[libvirt] [PATCH RFC 00/22] Support for walking backing chains on remote volumes (especially gluster)

This series adds support for waliking (detecting) of backing chain on
remote storage and implements the needed callbacks for the gluster
storage system via native library.

The last patch in the series mocks up path canonicalization on gluster as I'm
working on that but this series is getting tedious to rebase and handle so I'd
like to start reviews.

Peter Krempa (22):
  qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage
  storage: Store gluster volume name separately
  storage: Rework debugging of storage file access through storage
driver
  conf: Fix domain disk path iterator to work with networked storage
  storage: Add NONE protocol type for network disks
  storage: Add support for access to files using provided uid/gid
  storage: Add storage file API to read file headers
  storage: backend: Add unique id retrieval API
  storage: Add API to check accessibility of storage volumes
  storage: Move virStorageFileGetMetadata to the storage driver
  storage: Determine the local storage type right away
  test: storage: Initialize storage source to correct type
  storage: backend: Add possibility to suppress errors from backend
lookup
  storage: Switch metadata crawler to use storage driver to get unique
path
  storage: Switch metadata crawler to use storage driver to read headers
  storage: Switch metadata crawler to use storage driver file access
check
  storage: Add infrastructure to parse remote network backing names
  storage: Change to new backing store parser
  storage: Traverse backing chains of network disks
  qemu: Add support for networked disks for block pull/block rebase
  qemu: Add support for networked disks for block commit
  [DO NOT APPLY UPSTREAM] storage: gluster: mock up path
canonicalization

 cfg.mk|   2 +-
 src/Makefile.am   |   2 +
 src/conf/domain_conf.c|  70 +++-
 src/libvirt_private.syms  |   3 +-
 src/qemu/qemu_command.c   |  21 +-
 src/qemu/qemu_domain.c|  10 +-
 src/qemu/qemu_driver.c| 116 +--
 src/security/virt-aa-helper.c |   2 +
 src/storage/storage_backend.c |  16 +-
 src/storage/storage_backend.h |  22 +-
 src/storage/storage_backend_fs.c  | 127 +++-
 src/storage/storage_backend_gluster.c | 141 ++--
 src/storage/storage_driver.c  | 329 ++-
 src/storage/storage_driver.h  |  15 +-
 src/util/virstoragefile.c | 596 +-
 src/util/virstoragefile.h |  16 +-
 tests/Makefile.am |   7 +-
 tests/virstoragetest.c|  26 +-
 18 files changed, 1162 insertions(+), 359 deletions(-)

-- 
1.9.2

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

[libvirt] [PATCH RFC 16/22] storage: Switch metadata crawler to use storage driver file access check

Use virStorageFileAccess() to to check wether the file is accessible in
the main part of the metadata crawler.
---
 src/storage/storage_driver.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f2c548d..cf969c8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3076,13 +3076,6 @@ virFindBackingFile(const char *start, const char *path,
 goto cleanup;
 }

-if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid())  0) {
-virReportSystemError(errno,
- _(Cannot access backing file '%s'),
- combined);
-goto cleanup;
-}
-
 if (!(*canonical = canonicalize_file_name(combined))) {
 virReportSystemError(errno,
  _(Can't canonicalize path '%s'), path);
@@ -3124,6 +3117,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 if (virStorageFileInitAs(src, uid, gid)  0)
 return -1;

+if (virStorageFileAccess(src, F_OK)  0) {
+virReportSystemError(errno,
+ _(Cannot access backing file %s),
+ src-path);
+goto cleanup;
+}
+
 if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
 goto cleanup;

-- 
1.9.2

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

[libvirt] [PATCH RFC 14/22] storage: Switch metadata crawler to use storage driver to get unique path

Use the virStorageFileGetUniqueIdentifier() function to get a unique
identifier regardless of the target storage type instead of relying on
canonicalize_path().

A new function that checks wether we support a given image is introduced
to avoid errors for unimplemented backends.
---
 src/storage/storage_driver.c | 77 +++-
 1 file changed, 54 insertions(+), 23 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 163b402..cdf01e7 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2761,6 +2761,30 @@ virStorageFileIsInitialized(virStorageSourcePtr src)
 return !!src-drv;
 }

+
+static bool
+virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
+{
+int actualType = virStorageSourceGetActualType(src);
+virStorageFileBackendPtr backend;
+
+if (!src)
+return false;
+
+if (src-drv) {
+backend = src-drv-backend;
+} else {
+if (!(backend = virStorageFileBackendForTypeInternal(actualType,
+ src-protocol,
+ false)))
+return false;
+}
+
+return backend-storageFileGetUniqueIdentifier 
+   backend-storageFileReadHeader 
+   backend-storageFileAccess;
+}
+
 void
 virStorageFileDeinit(virStorageSourcePtr src)
 {
@@ -3077,7 +3101,6 @@ virFindBackingFile(const char *start, const char *path,
 /* Recursive workhorse for virStorageFileGetMetadata.  */
 static int
 virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
- const char *canonPath,
  uid_t uid, gid_t gid,
  bool allow_probe,
  virHashTablePtr cycle)
@@ -3085,49 +3108,63 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
 int fd;
 int ret = -1;
 struct stat st;
+const char *uniqueName;
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

-VIR_DEBUG(path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d,
-  src-path, canonPath, NULLSTR(src-relDir), src-format,
+VIR_DEBUG(path=%s dir=%s format=%d uid=%d gid=%d probe=%d,
+  src-path, NULLSTR(src-relDir), src-format,
   (int)uid, (int)gid, allow_probe);

-if (virHashLookup(cycle, canonPath)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(backing store for %s is self-referential),
-   src-path);
+/* exit if we can't load information about the current image */
+if (!virStorageFileSupportsBackingChainTraversal(src))
+return 0;
+
+if (virStorageFileInitAs(src, uid, gid)  0)
 return -1;
+
+if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
+goto cleanup;
+
+if (virHashLookup(cycle, uniqueName)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(backing store for %s (%s) is self-referential),
+   src-path, uniqueName);
+goto cleanup;
 }

-if (virHashAddEntry(cycle, canonPath, (void *)1)  0)
-return -1;
+if (virHashAddEntry(cycle, uniqueName, (void *)1)  0)
+goto cleanup;

 if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
 if ((fd = virFileOpenAs(src-path, O_RDONLY, 0, uid, gid, 0))  0) {
 virReportSystemError(-fd, _(Failed to open file '%s'),
  src-path);
-return -1;
+goto cleanup;
 }

 if (virStorageFileGetMetadataFromFDInternal(src, fd,
 backingFormat)  0) {
 VIR_FORCE_CLOSE(fd);
-return -1;
+goto cleanup;
 }

 if (VIR_CLOSE(fd)  0)
 VIR_WARN(could not close file %s, src-path);
 } else {
 /* TODO: currently we call this only for local storage */
-return 0;
+ret = 0;
+goto cleanup;
 }

 /* check whether we need to go deeper */
-if (!src-backingStoreRaw)
-return 0;
+if (!src-backingStoreRaw) {
+ret = 0;
+goto cleanup;
+}

 if (VIR_ALLOC(backingStore)  0)
-return -1;
+goto cleanup;

 if (VIR_STRDUP(backingStore-relPath, src-backingStoreRaw)  0)
 goto cleanup;
@@ -3174,7 +3211,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 backingStore-format = backingFormat;

 if (virStorageFileGetMetadataRecurse(backingStore,
- backingStore-path,
  uid, gid, allow_probe,
  cycle)  0) {
 /* if we fail somewhere midway, just accept and return a
@@ -3188,6 +3224,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,

[libvirt] [PATCH RFC 08/22] storage: backend: Add unique id retrieval API

Different protocols have different means to uniquely identify a storage
file. This patch implements a storage driver API to retrieve a unique
string describing a volume. The current implementation works for local
storage only and returns the canonical path of the volume.

To add caching support the local filesystem driver now has a private
structure holding the cached string, which is created only when it's
initially accessed.

This patch provides the implementation for local files only for start.
---
 src/storage/storage_backend.h|  3 +++
 src/storage/storage_backend_fs.c | 49 
 src/storage/storage_driver.c | 30 
 src/storage/storage_driver.h |  1 +
 4 files changed, 83 insertions(+)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 6be5ca7..5d71cde 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -195,6 +195,8 @@ typedef ssize_t
ssize_t max_len,
char **buf);

+typedef const char *
+(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);

 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);

@@ -211,6 +213,7 @@ struct _virStorageFileBackend {
 virStorageFileBackendInit backendInit;
 virStorageFileBackendDeinit backendDeinit;
 virStorageFileBackendReadHeader storageFileReadHeader;
+virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier;

 /* The following group of callbacks is expected to set errno
  * and return -1 on error. No libvirt error shall be reported */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 33551e7..1f436fa 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1339,6 +1339,14 @@ virStorageBackend virStorageBackendNetFileSystem = {
 };


+typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv;
+typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr;
+
+struct _virStorageFileBackendFsPriv {
+char *uid; /* unique file identifier (canonical path) */
+};
+
+
 static void
 virStorageFileBackendFileDeinit(virStorageSourcePtr src)
 {
@@ -1346,16 +1354,27 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src)
   virStorageTypeToString(virStorageSourceGetActualType(src)),
   src-path);

+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+VIR_FREE(priv-uid);
+VIR_FREE(priv);
 }


 static int
 virStorageFileBackendFileInit(virStorageSourcePtr src)
 {
+virStorageFileBackendFsPrivPtr priv = NULL;
+
 VIR_DEBUG(initializing FS storage file %p (%s:%s), src,
   virStorageTypeToString(virStorageSourceGetActualType(src)),
   src-path);

+if (VIR_ALLOC(priv)  0)
+return -1;
+
+src-drv-priv = priv;
+
 return 0;
 }

@@ -1403,6 +1422,23 @@ virStorageFileBackendFileReadHeader(virStorageSourcePtr 
src,
 }


+static const char *
+virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
+{
+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+if (!priv-uid) {
+if (!(priv-uid = canonicalize_file_name(src-path))) {
+virReportSystemError(errno, _(can't canonicalize path '%s'),
+ src-path);
+return NULL;
+}
+}
+
+return priv-uid;
+}
+
+
 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

@@ -1412,6 +1448,8 @@ virStorageFileBackend virStorageFileBackendFile = {
 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


@@ -1423,7 +1461,18 @@ virStorageFileBackend virStorageFileBackendBlock = {

 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };


+virStorageFileBackend virStorageFileBackendDir = {
+.type = VIR_STORAGE_TYPE_DIR,
+
+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
+.storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
+};
+
 #endif /* WITH_STORAGE_FS */
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 8da8386..2fb70f2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2960,3 +2960,33 @@ virStorageFileReadHeader(virStorageSourcePtr src,

 return ret;
 }
+
+
+/*
+ * virStorageFileGetUniqueIdentifier: Get a unique string describing the volume
+ *
+ * @src: file structure pointing to the file
+ *
+ * Returns a string uniquely describing a single volume

[libvirt] [PATCH RFC 13/22] storage: backend: Add possibility to suppress errors from backend lookup

Add a new function wrapper and tweak the storage file backend lookup
function so that it can be used without reporting error. This will be
useful in the metadata crawler code where we need silently break if
metadata retrieval is not supported for the current storage type.
---
 src/storage/storage_backend.c | 16 ++--
 src/storage/storage_backend.h |  4 +++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index e016cc8..380ca58 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1173,8 +1173,9 @@ virStorageBackendForType(int type)


 virStorageFileBackendPtr
-virStorageFileBackendForType(int type,
- int protocol)
+virStorageFileBackendForTypeInternal(int type,
+ int protocol,
+ bool report)
 {
 size_t i;

@@ -1188,6 +1189,9 @@ virStorageFileBackendForType(int type,
 }
 }

+if (!report)
+return NULL;
+
 if (type == VIR_STORAGE_TYPE_NETWORK) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(missing storage backend for network files 
@@ -1203,6 +1207,14 @@ virStorageFileBackendForType(int type,
 }


+virStorageFileBackendPtr
+virStorageFileBackendForType(int type,
+ int protocol)
+{
+return virStorageFileBackendForTypeInternal(type, protocol, true);
+}
+
+
 struct diskType {
 int part_table_type;
 unsigned short offset;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 56643fb..76c1afa 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -203,7 +203,9 @@ typedef int
int mode);

 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
-
+virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type,
+  int protocol,
+  bool report);


 struct _virStorageFileBackend {
-- 
1.9.2

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

[libvirt] [PATCH RFC 06/22] storage: Add support for access to files using provided uid/gid

To allow using the storage driver APIs to access files on various
storage sources in an universal fashion possibly on storage such as nfs
with root squash we'll need to store the desired uid/gid in the
metadata.

Add new initialisation API that will store the desired uid/gid and a
wrapper for the current use. Additionally add docs for the two APIs.
---
 src/storage/storage_backend.h |  3 +++
 src/storage/storage_driver.c  | 39 ++-
 src/storage/storage_driver.h  |  5 +++--
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 456b9d7..fcbb6da 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -169,6 +169,9 @@ typedef virStorageFileBackend *virStorageFileBackendPtr;
 struct _virStorageDriverData {
 virStorageFileBackendPtr backend;
 void *priv;
+
+uid_t uid;
+gid_t gid;
 };

 typedef int
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index db327f4..4c283fa 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2774,13 +2774,37 @@ virStorageFileDeinit(virStorageSourcePtr src)
 }


+/**
+ * virStorageFileInitAs:
+ *
+ * @src: storage source definition
+ * @uid: uid to access the file as, -1 for current uid
+ * @gid: gid to access the file as, -1 for current gid
+ *
+ * Initialize a storage source to be used with storage driver. Use the provided
+ * uid and gid if possible for the operations.
+ *
+ * Returns 0 if the storage file was successfully initialized, -1 if the
+ * initialization failed. Libvirt error is reported.
+ */
 int
-virStorageFileInit(virStorageSourcePtr src)
+virStorageFileInitAs(virStorageSourcePtr src,
+ uid_t uid, gid_t gid)
 {
 int actualType = virStorageSourceGetActualType(src);
 if (VIR_ALLOC(src-drv)  0)
 return -1;

+if (uid == (uid_t) -1)
+src-drv-uid = geteuid();
+else
+src-drv-uid = uid;
+
+if (gid == (gid_t) -1)
+src-drv-gid = getegid();
+else
+src-drv-gid = gid;
+
 if (!(src-drv-backend = virStorageFileBackendForType(actualType,
src-protocol)))
 goto error;
@@ -2798,6 +2822,19 @@ virStorageFileInit(virStorageSourcePtr src)


 /**
+ * virStorageFileInit:
+ *
+ * See virStorageFileInitAs. The file is initialized to be accessed by the
+ * current user.
+ */
+int
+virStorageFileInit(virStorageSourcePtr src)
+{
+return virStorageFileInitAs(src, (uid_t) -1, (gid_t) -1);
+}
+
+
+/**
  * virStorageFileCreate: Creates an empty storage file via storage driver
  *
  * @src: file structure pointing to the file
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index fb03870..49be999 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -29,8 +29,9 @@
 # include storage_conf.h
 # include virstoragefile.h

-int
-virStorageFileInit(virStorageSourcePtr src);
+int virStorageFileInit(virStorageSourcePtr src);
+int virStorageFileInitAs(virStorageSourcePtr src,
+ uid_t uid, gid_t gid);
 void virStorageFileDeinit(virStorageSourcePtr src);

 int virStorageFileCreate(virStorageSourcePtr src);
-- 
1.9.2

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

[libvirt] [PATCH RFC 09/22] storage: Add API to check accessibility of storage volumes

Add a storage driver API equivalent ot the access() function.
Implementations for the filesystem and gluster backends are provided.
---
 src/storage/storage_backend.h |  5 +
 src/storage/storage_backend_fs.c  | 13 +
 src/storage/storage_backend_gluster.c | 11 +++
 src/storage/storage_driver.c  | 24 
 src/storage/storage_driver.h  |  1 +
 5 files changed, 54 insertions(+)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 5d71cde..56643fb 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -198,6 +198,10 @@ typedef ssize_t
 typedef const char *
 (*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);

+typedef int
+(*virStorageFileBackendAccess)(virStorageSourcePtr src,
+   int mode);
+
 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);


@@ -220,6 +224,7 @@ struct _virStorageFileBackend {
 virStorageFileBackendCreate storageFileCreate;
 virStorageFileBackendUnlink storageFileUnlink;
 virStorageFileBackendStat   storageFileStat;
+virStorageFileBackendAccess storageFileAccess;
 };

 #endif /* __VIR_STORAGE_BACKEND_H__ */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1f436fa..85264a7 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1439,6 +1439,15 @@ 
virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
 }


+static int
+virStorageFileBackendFileAccess(virStorageSourcePtr src,
+int mode)
+{
+return virFileAccessibleAs(src-path, mode,
+   src-drv-uid, src-drv-gid);
+}
+
+
 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

@@ -1448,6 +1457,7 @@ virStorageFileBackend virStorageFileBackendFile = {
 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+.storageFileAccess = virStorageFileBackendFileAccess,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
@@ -1461,6 +1471,7 @@ virStorageFileBackend virStorageFileBackendBlock = {

 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+.storageFileAccess = virStorageFileBackendFileAccess,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
@@ -1472,6 +1483,8 @@ virStorageFileBackend virStorageFileBackendDir = {
 .backendInit = virStorageFileBackendFileInit,
 .backendDeinit = virStorageFileBackendFileDeinit,

+.storageFileAccess = virStorageFileBackendFileAccess,
+
 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };

diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index badffae..1a844c9 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -703,6 +703,16 @@ virStorageFileBackendGlusterReadHeader(virStorageSourcePtr 
src,
 }


+static int
+virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
+   int mode)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+
+return glfs_access(priv-vol, src-path, mode);
+}
+
+
 virStorageFileBackend virStorageFileBackendGluster = {
 .type = VIR_STORAGE_TYPE_NETWORK,
 .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER,
@@ -713,4 +723,5 @@ virStorageFileBackend virStorageFileBackendGluster = {
 .storageFileUnlink = virStorageFileBackendGlusterUnlink,
 .storageFileStat = virStorageFileBackendGlusterStat,
 .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
+.storageFileAccess = virStorageFileBackendGlusterAccess,
 };
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 2fb70f2..dd58400 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2990,3 +2990,27 @@ virStorageFileGetUniqueIdentifier(virStorageSourcePtr 
src)

 return src-drv-backend-storageFileGetUniqueIdentifier(src);
 }
+
+
+/**
+ * virStorageFileAccess: Check accessability of a storage file
+ *
+ * @src: storage file to check access permissions
+ * @mode: accessibility check options (see man 2 access)
+ *
+ * Returns 0 on success, -1 on error and sets errno. No libvirt
+ * error is reported. Returns -2 if the operation isn't supported
+ * by libvirt storage backend.
+ */
+int
+virStorageFileAccess(virStorageSourcePtr src,
+ int mode)
+{
+if (!virStorageFileIsInitialized(src) ||
+!src-drv-backend-storageFileAccess) {
+errno = ENOSYS;
+return -2;
+}
+
+return src-drv-backend-storageFileAccess(src, mode);
+}
diff --git a/src/storage/storage_driver.h

[libvirt] [PATCH RFC 17/22] storage: Add infrastructure to parse remote network backing names

Add parsers for relative and absolute backing names for local and remote
storage files.

This parser parses relative paths as relative to their parents and
absolute paths according to the protocol or local access.

For remote storage volumes, all URI based backing file names are
supported and for the qemu colon syntax the NBD protocol is supported.
---
 src/libvirt_private.syms  |   1 +
 src/util/virstoragefile.c | 360 ++
 src/util/virstoragefile.h |   4 +
 3 files changed, 365 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 90bd48c..8342ec9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1867,6 +1867,7 @@ virStorageSourceClear;
 virStorageSourceClearBackingStore;
 virStorageSourceFree;
 virStorageSourceGetActualType;
+virStorageSourceNewFromBacking;
 virStorageSourcePoolDefFree;
 virStorageSourcePoolModeTypeFromString;
 virStorageSourcePoolModeTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 365320b..0d155ca 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -38,6 +38,8 @@
 #include virendian.h
 #include virstring.h
 #include virutil.h
+#include viruri.h
+#include dirname.h
 #if HAVE_SYS_SYSCALL_H
 # include sys/syscall.h
 #endif
@@ -658,6 +660,19 @@ virStorageIsFile(const char *backing)
 }


+static bool
+virStorageIsRelative(const char *backing)
+{
+if (backing[0] == '/')
+return false;
+
+if (!virStorageIsFile(backing))
+return false;
+
+return true;
+}
+
+
 static int
 virStorageFileProbeFormatFromBuf(const char *path,
  char *buf,
@@ -1558,3 +1573,348 @@ virStorageSourceFree(virStorageSourcePtr def)
 virStorageSourceClear(def);
 VIR_FREE(def);
 }
+
+
+static virStorageSourcePtr
+virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
+   const char *rel)
+{
+char *dirname = NULL;
+virStorageSourcePtr ret;
+
+if (VIR_ALLOC(ret)  0)
+return NULL;
+
+ret-backingRelative = true;
+
+/* XXX Once we get rid of the need to use canonical names in path, we will 
be
+ * able to use mdir_name on parent-path instead of using parent-relDir */
+if (virAsprintf(ret-path, %s/%s, parent-relDir, rel)  0)
+goto error;
+
+if (virStorageSourceGetActualType(parent) != VIR_STORAGE_TYPE_NETWORK) {
+ret-type = VIR_STORAGE_TYPE_FILE;
+
+/* XXX store the relative directory name for test's sake */
+if (!(ret-relDir = mdir_name(ret-path))) {
+virReportOOMError();
+goto error;
+}
+
+/* XXX we don't currently need to store the canonical path but the
+ * change would break the test suite. Get rid of this later */
+char *tmp = ret-path;
+if (!(ret-path = canonicalize_file_name(tmp))) {
+ret-path = tmp;
+tmp = NULL;
+}
+VIR_FREE(tmp);
+} else {
+ret-type = VIR_STORAGE_TYPE_NETWORK;
+
+/* copy the host network part */
+ret-protocol = parent-protocol;
+if (!(ret-hosts = virStorageNetHostDefCopy(parent-nhosts, 
parent-hosts)))
+goto error;
+ret-nhosts = parent-nhosts;
+
+if (VIR_STRDUP(ret-volume, parent-volume)  0)
+goto error;
+
+/* XXX store the relative directory name for test's sake */
+if (!(ret-relDir = mdir_name(ret-path))) {
+virReportOOMError();
+goto error;
+}
+}
+
+ cleanup:
+VIR_FREE(dirname);
+return ret;
+
+ error:
+virStorageSourceFree(ret);
+ret = NULL;
+goto cleanup;
+}
+
+
+static int
+virStorageSourceParseBackingURI(virStorageSourcePtr src,
+const char *path)
+{
+virURIPtr uri = NULL;
+char **scheme = NULL;
+int ret = -1;
+
+if (!(uri = virURIParse(path))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(failed to parse backing file location '%s'),
+   path);
+goto cleanup;
+}
+
+if (!(scheme = virStringSplit(uri-scheme, +, 2)))
+goto cleanup;
+
+if (!scheme[0] ||
+(src-protocol = virStorageNetProtocolTypeFromString(scheme[0]))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(invalid backing protocol '%s'),
+   NULLSTR(scheme[0]));
+goto cleanup;
+}
+
+if (scheme[1] 
+(src-hosts-transport = 
virStorageNetHostTransportTypeFromString(scheme[1]))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(invalid protocol transport type '%s'),
+   scheme[1]);
+goto cleanup;
+}
+
+/* handle socket stored as a query */
+if (uri-query) {
+if (VIR_STRDUP(src-hosts-socket, STRSKIP(uri-query, socket=))  0)
+goto cleanup;
+}
+
+/* XXX We

[libvirt] [PATCH RFC 19/22] storage: Traverse backing chains of network disks

Now we don't need to skip backing chain detection for remote disks.
---
 src/qemu/qemu_domain.c   |  8 +++-
 src/storage/storage_driver.c | 18 ++
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7ea9032..cd37ac4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2414,12 +2414,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
 int ret = 0;
 uid_t uid;
 gid_t gid;
-const char *src = virDomainDiskGetSource(disk);
-int type = virDomainDiskGetType(disk);
+int type = virStorageSourceGetActualType(disk-src);

-if (!src ||
-type == VIR_STORAGE_TYPE_NETWORK ||
-type == VIR_STORAGE_TYPE_VOLUME)
+if (type != VIR_STORAGE_TYPE_NETWORK 
+!disk-src.path)
 goto cleanup;

 if (disk-src.backingStore) {
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ac49553..1486b72 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3162,19 +3162,13 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 if (!(cycle = virHashCreate(5, NULL)))
 return -1;

-if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
-if (!src-relPath 
-VIR_STRDUP(src-relPath, src-path)  0)
-goto cleanup;
+if (!src-relPath 
+VIR_STRDUP(src-relPath, src-path)  0)
+goto cleanup;

-if (!src-relDir 
-!(src-relDir = mdir_name(src-path))) {
-virReportOOMError();
-goto cleanup;
-}
-} else {
-/* TODO: currently unimplemented for non-local storage */
-ret = 0;
+if (!src-relDir 
+!(src-relDir = mdir_name(src-path))) {
+virReportOOMError();
 goto cleanup;
 }

-- 
1.9.2

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

[libvirt] [PATCH RFC 10/22] storage: Move virStorageFileGetMetadata to the storage driver

My future work will modify the metadata crawler function to use the
storage driver file APIs to access the files instead of accessing them
directly so that we will be able to request the metadata for remote
files too. To avoid linking the storage driver to every helper file
using the utils code, the backing chain traversal function needs to be
moved to the storage driver source.

Additionally the virt-aa-helper and virstoragetest programs need to be
linked with the storage driver as a result of this change.
---
 cfg.mk|   2 +-
 src/Makefile.am   |   2 +
 src/libvirt_private.syms  |   2 +-
 src/qemu/qemu_domain.c|   2 +
 src/security/virt-aa-helper.c |   2 +
 src/storage/storage_driver.c  | 233 ++
 src/storage/storage_driver.h  |   5 +
 src/util/virstoragefile.c | 233 +-
 src/util/virstoragefile.h |   7 +-
 tests/Makefile.am |   7 +-
 tests/virstoragetest.c|   2 +
 11 files changed, 258 insertions(+), 239 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 3f4bba0..cfe5fcc 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -774,7 +774,7 @@ sc_prohibit_cross_inclusion:
access/ | conf/) safe=($$dir|conf|util);; \
locking/) safe=($$dir|util|conf|rpc);;\
cpu/| network/| node_device/| rpc/| security/| storage/)\
- safe=($$dir|util|conf);;\
+ safe=($$dir|util|conf|storage);;\
xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);;   \
*) safe=($$dir|$(mid_dirs)|util);;\
  esac; \
diff --git a/src/Makefile.am b/src/Makefile.am
index e9dc9e0..072e761 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2564,8 +2564,10 @@ virt_aa_helper_LDFLAGS = \
$(PIE_LDFLAGS) \
$(NULL)
 virt_aa_helper_LDADD = \
+   libvirt.la  \
libvirt_conf.la \
libvirt_util.la \
+   libvirt_driver_storage_impl.la  \
../gnulib/lib/libgnu.la
 if WITH_DTRACE_PROBES
 virt_aa_helper_LDADD += libvirt_probes.lo
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index afda71f..78221aa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1847,9 +1847,9 @@ virStorageFileFeatureTypeToString;
 virStorageFileFormatTypeFromString;
 virStorageFileFormatTypeToString;
 virStorageFileGetLVMKey;
-virStorageFileGetMetadata;
 virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
+virStorageFileGetMetadataFromFDInternal;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
 virStorageFileParseChainIndex;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1301f64..7ea9032 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -39,6 +39,8 @@
 #include virstoragefile.h
 #include virstring.h

+#include storage/storage_driver.h
+
 #include sys/time.h
 #include fcntl.h

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 32fc04a..bf540b4 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -55,6 +55,8 @@
 #include virrandom.h
 #include virstring.h

+#include storage/storage_driver.h
+
 #define VIR_FROM_THIS VIR_FROM_SECURITY

 static char *progname;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index dd58400..25772e8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -49,6 +49,7 @@
 #include configmake.h
 #include virstring.h
 #include viraccessapicheck.h
+#include dirname.h

 #define VIR_FROM_THIS VIR_FROM_STORAGE

@@ -3014,3 +3015,235 @@ virStorageFileAccess(virStorageSourcePtr src,

 return src-drv-backend-storageFileAccess(src, mode);
 }
+
+
+/**
+ * Given a starting point START (a directory containing the original
+ * file, if the original file was opened via a relative path; ignored
+ * if NAME is absolute), determine the location of the backing file
+ * NAME (possibly relative), and compute the relative DIRECTORY
+ * (optional) and CANONICAL (mandatory) location of the backing file.
+ * Return 0 on success, negative on error.
+ */
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
+virFindBackingFile(const char *start, const char *path,
+   char **directory, char **canonical)
+{
+/* FIXME - when we eventually allow non-raw network devices, we
+ * must ensure that we handle backing files the same way as qemu.
+ * For a qcow2 top file of gluster://server/vol/img, qemu treats
+ * the relative backing file 'rel' as meaning
+ * 'gluster://server/vol/rel', while

[libvirt] LSN-2014-0003: Unsafe parsing of XML documents allows arbitrary file read

Libvirt Security Notice: LSN-2014-0003
==

   Summary: Unsafe parsing of XML documents allows arbitrary
file read
   Reported on: 20140411
  Published on: 20140506
  Fixed on: 20140506
   Reported by: Daniel P. Berrange berra...@redhat.com
Richard Jones rjo...@redhat.com
Patched by: Daniel P. Berrange berra...@redhat.com
  See also: CVE-2014-0179

Description
---

When parsing XML documents, libvirt passes the XML_PARSE_NOENT flag
to libxml2 which instructs it to expand all entities in the XML
document during parsing. This can be used to insert the contents of
host OS files in the resulting parsed content. Although the flaw was
introduced in 0.0.5, it was dormant having no ill effects, since the
APIs involved all required the user to authenticate with privileges
equivalent to root. In version 0.7.5 or later the
virConnectCompareCPU / virConnectBaselineCPU methods activate the
dormant bug, allowing for denial of service. In version 1.0.0 or
later, if the admin opts in to using the new fine grained access
control feature, there is potential for unprivileged information
disclosure.

Impact
--

A malicious user can pass libvirt an XML document which contains an
entity that points to an arbitrary file on the host. When libvirt
parses this document, it will insert the contents of that host file,
which could allow the user to read the contents of files that they
otherwise do not have permission to view. It also has the potential
to cause a denial of service / indefinite hang of libvirt, if the
entity points to a named pipe with no writer connected or certain
proc files. If the libvirt installation is not using fine grained
access control then virConnectCompareCPU and virConnectBaselineCPU
APIs can be used by a read-only user to inflict a denial of service
attack. If the libvirt installation is using fine grained access
control, then as well as the denial of service attack, one or more
of the following APIs can be used for information disclosure of
files: virDomainDefineXML, virNetworkCreateXML, virNetworkDefineXML,
virStoragePoolCreateXML, virStoragePoolDefineXML,
virStorageVolCreateXML, virDomainCreateXML, virNodeDeviceCreateXML,
virInterfaceDefineXML, virStorageVolCreateXMLFrom,
virConnectDomainXMLFromNative, virConnectDomainXMLToNative,
virSecretDefineXML, virNWFilterDefineXML,
virDomainSnapshotCreateXML, virDomainSaveImageDefineXML,
virDomainCreateXMLWithFiles, virConnectCompareCPU,
virConnectBaselineCPU.

Workaround
--

Stop use of the fine grained access control mechanism, and restrict
access to all the libvirt TCP/UNIX sockets to only trusted
authenticated users. Simply denying access to the affected APIs in
the access control policy is insufficient to mitigate the bug, since
the XML document typically needs to be parsed before the access
control check is applied in order to extra the UUID/name of the
object to check. Access to the readonly libvirt socket must also be
revoked

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git
  http://libvirt.org/git/?p=libvirt.git

  Branch: master
   Broken in: v0.7.5
   Broken in: v0.7.6
   Broken in: v0.7.7
   Broken in: v0.8.0
   Broken in: v0.8.1
   Broken in: v0.8.2
   Broken in: v0.8.3
   Broken in: v0.8.4
   Broken in: v0.8.5
   Broken in: v0.8.6
   Broken in: v0.8.7
   Broken in: v0.8.8
   Broken in: v0.9.0
   Broken in: v0.9.1
   Broken in: v0.9.2
   Broken in: v0.9.3
   Broken in: v0.9.4
   Broken in: v0.9.5
   Broken in: v0.9.6
   Broken in: v0.9.7
   Broken in: v0.9.8
   Broken in: v0.9.9
   Broken in: v0.9.10
   Broken in: v0.9.11
   Broken in: v0.9.12
   Broken in: v0.9.13
   Broken in: v1.0.0
   Broken in: v1.0.1
   Broken in: v1.0.2
   Broken in: v1.0.3
   Broken in: v1.0.4
   Broken in: v1.0.5
   Broken in: v1.0.6
   Broken in: v1.1.0
   Broken in: v1.1.1
   Broken in: v1.1.2
   Broken in: v1.1.3
   Broken in: v1.1.4
   Broken in: v1.2.0
   Broken in: v1.2.1
   Broken in: v1.2.2
   Broken in: v1.2.3
   Broken by: 77e8b6c62c48b6346bbdb2df3e0d925852c6bf3e
   Broken by: 387941fb626d9362835aa216b4a871e18268f649
   Broken by: 0b7d2ae653f583825f6d83bfb0744673648a9833
   Broken by: ed3bac713c3cfc055ef551cbfe92a061084382c3
Fixed by: d6b27d3e4c40946efa79e91d134616b41b1666c4

  Branch: v0.9.6-maint
   Broken in: v0.9.6.1
   Broken in: v0.9.6.2
   Broken in: v0.9.6.3
   Broken in: v0.9.6.4
   Broken by: 77e8b6c62c48b6346bbdb2df3e0d925852c6bf3e
   Broken by: 387941fb626d9362835aa216b4a871e18268f649
   Broken by: 0b7d2ae653f583825f6d83bfb0744673648a9833
Fixed by: be7a5de9d0c406f36efae3230e1743c613ad6945

  Branch: v0.9.11-maint
   Broken in: v0.9.11.1
   Broken in: v0.9.11.2
   Broken in: v0.9.11.3
   Broken in: v0.9.11.4
   Broken in: v0.9.11.5
   Broken in: v0.9.11.6
   Broken in: v0.9.11.7
   Broken in: v0.9.11.8
   Broken in: v0.9.11.9
   Broken

[libvirt] [PATCH RFC 05/22] storage: Add NONE protocol type for network disks

Currently the protocol type with index 0 was NBD which made it hard to
distinguish wether the protocol type was actually assigned. Add a new
protocol type with index 0 to distinguish it explicitly.
---
 src/qemu/qemu_command.c   | 2 ++
 src/qemu/qemu_driver.c| 3 +++
 src/util/virstoragefile.c | 1 +
 src/util/virstoragefile.h | 1 +
 4 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c0f2fb6..48eeae0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3582,6 +3582,7 @@ qemuNetworkDriveGetPort(int protocol,

 case VIR_STORAGE_NET_PROTOCOL_RBD:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
+case VIR_STORAGE_NET_PROTOCOL_NONE:
 /* not aplicable */
 return -1;
 }
@@ -3798,6 +3799,7 @@ qemuBuildNetworkDriveURI(int protocol,


 case VIR_STORAGE_NET_PROTOCOL_LAST:
+case VIR_STORAGE_NET_PROTOCOL_NONE:
 goto cleanup;
 }

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ac0de1e..b0a79e4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12338,6 +12338,7 @@ 
qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)

 case VIR_STORAGE_TYPE_NETWORK:
 switch ((virStorageNetProtocol) disk-src.protocol) {
+case VIR_STORAGE_NET_PROTOCOL_NONE:
 case VIR_STORAGE_NET_PROTOCOL_NBD:
 case VIR_STORAGE_NET_PROTOCOL_RBD:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
@@ -12403,6 +12404,7 @@ 
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
 return 0;

+case VIR_STORAGE_NET_PROTOCOL_NONE:
 case VIR_STORAGE_NET_PROTOCOL_NBD:
 case VIR_STORAGE_NET_PROTOCOL_RBD:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
@@ -12545,6 +12547,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr 
conn,

 case VIR_STORAGE_TYPE_NETWORK:
 switch ((virStorageNetProtocol) disk-src.protocol) {
+case VIR_STORAGE_NET_PROTOCOL_NONE:
 case VIR_STORAGE_NET_PROTOCOL_NBD:
 case VIR_STORAGE_NET_PROTOCOL_RBD:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cf270f6..73cef3c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -72,6 +72,7 @@ VIR_ENUM_IMPL(virStorageFileFeature,
   )

 VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST,
+  none,
   nbd,
   rbd,
   sheepdog,
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3777d66..95f2e45 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -118,6 +118,7 @@ struct _virStorageTimestamps {

 /* Information related to network storage */
 typedef enum {
+VIR_STORAGE_NET_PROTOCOL_NONE,
 VIR_STORAGE_NET_PROTOCOL_NBD,
 VIR_STORAGE_NET_PROTOCOL_RBD,
 VIR_STORAGE_NET_PROTOCOL_SHEEPDOG,
-- 
1.9.2

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

[libvirt] [PATCH RFC 07/22] storage: Add storage file API to read file headers

Add storage driver based functions to access headers of storage files
for metadata extraction. Along with this patch a local filesystem and
gluster via libgfapi implementation is provided. The gluster
implementation is based on code of the saferead_lim function.
---
 src/storage/storage_backend.h |  7 
 src/storage/storage_backend_fs.c  | 30 
 src/storage/storage_backend_gluster.c | 66 +++
 src/storage/storage_driver.c  | 40 +
 src/storage/storage_driver.h  |  3 ++
 5 files changed, 146 insertions(+)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index fcbb6da..6be5ca7 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -190,6 +190,12 @@ typedef int
 (*virStorageFileBackendStat)(virStorageSourcePtr src,
  struct stat *st);

+typedef ssize_t
+(*virStorageFileBackendReadHeader)(virStorageSourcePtr src,
+   ssize_t max_len,
+   char **buf);
+
+
 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);


@@ -204,6 +210,7 @@ struct _virStorageFileBackend {
  * error on failure. */
 virStorageFileBackendInit backendInit;
 virStorageFileBackendDeinit backendDeinit;
+virStorageFileBackendReadHeader storageFileReadHeader;

 /* The following group of callbacks is expected to set errno
  * and return -1 on error. No libvirt error shall be reported */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 3d088ba..33551e7 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1375,6 +1375,34 @@ virStorageFileBackendFileStat(virStorageSourcePtr src,
 }


+static ssize_t
+virStorageFileBackendFileReadHeader(virStorageSourcePtr src,
+ssize_t max_len,
+char **buf)
+{
+int fd = -1;
+ssize_t ret = -1;
+
+if ((fd = virFileOpenAs(src-path, O_RDONLY, 0,
+src-drv-uid, src-drv-gid, 0))  0) {
+virReportSystemError(-fd, _(Failed to open file '%s'),
+ src-path);
+return -1;
+}
+
+if ((ret = virFileReadHeaderFD(fd, max_len, buf))  0) {
+virReportSystemError(errno,
+ _(cannot read header '%s'), src-path);
+goto cleanup;
+}
+
+ cleanup:
+VIR_FORCE_CLOSE(fd);
+
+return ret;
+}
+
+
 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

@@ -1383,6 +1411,7 @@ virStorageFileBackend virStorageFileBackendFile = {

 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
+.storageFileReadHeader = virStorageFileBackendFileReadHeader,
 };


@@ -1393,6 +1422,7 @@ virStorageFileBackend virStorageFileBackendBlock = {
 .backendDeinit = virStorageFileBackendFileDeinit,

 .storageFileStat = virStorageFileBackendFileStat,
+.storageFileReadHeader = virStorageFileBackendFileReadHeader,
 };


diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index e578e73..badffae 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -638,6 +638,71 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
 }


+static ssize_t
+virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
+   ssize_t max_len,
+   char **buf)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+glfs_fd_t *fd = NULL;
+size_t alloc = 0;
+size_t size = 0;
+int save_errno;
+ssize_t ret = -1;
+
+*buf = NULL;
+
+if (!(fd = glfs_open(priv-vol, src-path, O_RDONLY))) {
+virReportSystemError(errno, _(Failed to open file '%s'),
+ src-path);
+goto cleanup;
+}
+
+/* code below is shamelesly stolen from saferead_lim */
+for (;;) {
+int count;
+int requested;
+
+if (size + BUFSIZ + 1  alloc) {
+alloc += alloc / 2;
+if (alloc  size + BUFSIZ + 1)
+alloc = size + BUFSIZ + 1;
+
+if (VIR_REALLOC_N(*buf, alloc)  0) {
+save_errno = errno;
+break;
+}
+}
+
+/* Ensure that (size + requested = max_len); */
+requested = MIN(size  max_len ? max_len - size : 0,
+alloc - size - 1);
+count = glfs_read(fd, *buf + size, requested, 0);
+size += count;
+
+if (count != requested || requested == 0) {
+save_errno = errno;
+if (count  0) {
+virReportSystemError(errno,
+ _(cannot read header '%s'), src-path);
+break;

[libvirt] [PATCH RFC 02/22] storage: Store gluster volume name separately

The gluster volume name was previously stored as part of the source path
string. This isn't a good idea when we want to start to deal with
resolving of relative paths on gluster as a storage volume can't
actually be changed by ../-ing it.

Parse and store the volume name separately for gluster storage volumes
and use the newly stored variable appropriately.
---
 src/conf/domain_conf.c| 33 -
 src/qemu/qemu_command.c   | 19 ++-
 src/storage/storage_backend_gluster.c | 27 ---
 src/util/virstoragefile.c |  1 +
 src/util/virstoragefile.h |  1 +
 5 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c3bdad..3cb6693 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5002,6 +5002,27 @@ virDomainDiskSourceParse(xmlNodePtr node,
 goto cleanup;
 }

+/* for historical reasons the volume name for gluster volume is stored
+ * as a part of the path. This is hard to work with when dealing with
+ * relative names. Split out the volume into a separate variable */
+if (src-path  src-protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
+char *tmp;
+if (!(tmp = strchr(src-path, '/')) ||
+tmp == src-path) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(missing volume name or file name in 
+ gluster source path '%s'), src-path);
+goto cleanup;
+}
+
+src-volume = src-path;
+
+if (VIR_STRDUP(src-path, tmp)  0)
+goto cleanup;
+
+tmp[0] = '\0';
+}
+
 child = node-children;
 while (child != NULL) {
 if (child-type == XML_ELEMENT_NODE 
@@ -14841,6 +14862,7 @@ virDomainDiskSourceFormat(virBufferPtr buf,
   unsigned int flags)
 {
 size_t n;
+char *path = NULL;
 const char *startupPolicy = NULL;

 if (policy)
@@ -14876,7 +14898,16 @@ virDomainDiskSourceFormat(virBufferPtr buf,
 case VIR_STORAGE_TYPE_NETWORK:
 virBufferAsprintf(buf, source protocol='%s',
   
virStorageNetProtocolTypeToString(src-protocol));
-virBufferEscapeString(buf,  name='%s', src-path);
+
+
+if (src-volume) {
+if (virAsprintf(path, %s%s, src-volume, src-path)  0)
+return -1;
+}
+
+virBufferEscapeString(buf,  name='%s', path ? path : src-path);
+
+VIR_FREE(path);

 if (src-nhosts == 0) {
 virBufferAddLit(buf, /\n);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c1e17d..c0f2fb6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3594,6 +3594,7 @@ qemuNetworkDriveGetPort(int protocol,
 static char *
 qemuBuildNetworkDriveURI(int protocol,
  const char *src,
+ const char *volume,
  size_t nhosts,
  virStorageNetHostDefPtr hosts,
  const char *username,
@@ -3693,11 +3694,18 @@ qemuBuildNetworkDriveURI(int protocol,
 if ((uri-port = qemuNetworkDriveGetPort(protocol, hosts-port))  
0)
 goto cleanup;

-if (src 
-virAsprintf(uri-path, %s%s,
-src[0] == '/' ?  : /,
-src)  0)
-goto cleanup;
+if (src) {
+if (volume) {
+if (virAsprintf(uri-path, /%s%s,
+volume, src)  0)
+goto cleanup;
+} else {
+if (virAsprintf(uri-path, %s%s,
+src[0] == '/' ?  : /,
+src)  0)
+goto cleanup;
+}
+}

 if (hosts-socket 
 virAsprintf(uri-query, socket=%s, hosts-socket)  0)
@@ -3859,6 +3867,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 case VIR_STORAGE_TYPE_NETWORK:
 if (!(*source = qemuBuildNetworkDriveURI(src-protocol,
  src-path,
+ src-volume,
  src-nhosts,
  src-hosts,
  username,
diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index 2f0592f..8679d96 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -533,8 +533,6 @@ typedef virStorageFileBackendGlusterPriv 
*virStorageFileBackendGlusterPrivPtr;

[libvirt] [PATCH RFC 21/22] qemu: Add support for networked disks for block commit

Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
 src/qemu/qemu_driver.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9cf5d5..1188300 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15355,6 +15355,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
 unsigned int baseIndex = 0;
 const char *top_parent = NULL;
 bool clean_access = false;
+char *topPath = NULL;
+char *basePath = NULL;

 virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);

@@ -15441,6 +15443,22 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE)  0))
 goto endjob;

+if (top  !topIndex) {
+if (VIR_STRDUP(topPath, top)  0)
+goto endjob;
+} else {
+if (qemuGetDriveSourceString(topSource, NULL, topPath)  0)
+goto endjob;
+}
+
+if (base  !baseIndex) {
+if (VIR_STRDUP(basePath, base)  0)
+goto endjob;
+} else {
+if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
+goto endjob;
+}
+
 /* Start the commit operation.  Pass the user's original spelling,
  * if any, through to qemu, since qemu may behave differently
  * depending on whether the input was specified as relative or
@@ -15448,8 +15466,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
  * thing if the user specified a relative name). */
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorBlockCommit(priv-mon, device,
- top  !topIndex ? top : topSource-path,
- base  !baseIndex ? base : baseSource-path,
+ topPath, basePath,
  bandwidth);
 qemuDomainObjExitMonitor(driver, vm);

@@ -15467,6 +15484,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
 vm = NULL;

  cleanup:
+VIR_FREE(topPath);
+VIR_FREE(basePath);
 VIR_FREE(device);
 if (vm)
 virObjectUnlock(vm);
-- 
1.9.2

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

[libvirt] [PATCH RFC 15/22] storage: Switch metadata crawler to use storage driver to read headers

Use virStorageFileReadHeader() to read headers of storage files possibly
on remote storate to retrieve the image metadata.

The backend information is now parsed by
virStorageFileGetMetadataInternal which is now exported from the util
source and virStorageFileGetMetadataFromFDInternal now doesn't need to
be exported.
---
 src/libvirt_private.syms |  2 +-
 src/storage/storage_driver.c | 26 +++---
 src/util/virstoragefile.c|  5 ++---
 src/util/virstoragefile.h|  9 ++---
 4 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 78221aa..90bd48c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1849,7 +1849,7 @@ virStorageFileFormatTypeToString;
 virStorageFileGetLVMKey;
 virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
-virStorageFileGetMetadataFromFDInternal;
+virStorageFileGetMetadataInternal;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
 virStorageFileParseChainIndex;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cdf01e7..f2c548d 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3105,10 +3105,11 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
  bool allow_probe,
  virHashTablePtr cycle)
 {
-int fd;
 int ret = -1;
 struct stat st;
 const char *uniqueName;
+char *buf = NULL;
+ssize_t headerLen;
 virStorageSourcePtr backingStore = NULL;
 int backingFormat;

@@ -3136,26 +3137,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
 if (virHashAddEntry(cycle, uniqueName, (void *)1)  0)
 goto cleanup;

-if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
-if ((fd = virFileOpenAs(src-path, O_RDONLY, 0, uid, gid, 0))  0) {
-virReportSystemError(-fd, _(Failed to open file '%s'),
- src-path);
-goto cleanup;
-}
-
-if (virStorageFileGetMetadataFromFDInternal(src, fd,
-backingFormat)  0) {
-VIR_FORCE_CLOSE(fd);
-goto cleanup;
-}
+if ((headerLen = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
+  buf))  0)
+goto cleanup;

-if (VIR_CLOSE(fd)  0)
-VIR_WARN(could not close file %s, src-path);
-} else {
-/* TODO: currently we call this only for local storage */
-ret = 0;
+if (virStorageFileGetMetadataInternal(src, buf, headerLen,
+  backingFormat)  0)
 goto cleanup;
-}

 /* check whether we need to go deeper */
 if (!src-backingStoreRaw) {
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 347c2c3..365320b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -738,8 +738,7 @@ qcow2GetFeatures(virBitmapPtr *features,
  * with information about the file and its backing store. Return format
  * of the backing store as BACKING_FORMAT. PATH and FORMAT have to be
  * pre-populated in META */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(4)
+int
 virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
   char *buf,
   size_t len,
@@ -960,7 +959,7 @@ virStorageFileGetMetadataFromBuf(const char *path,


 /* Internal version that also supports a containing directory name.  */
-int
+static int
 virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
 int fd,
 int *backingFormat)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index ae27936..936b0ac 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -263,9 +263,12 @@ struct _virStorageSource {

 int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);

-int virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
-int fd,
-int *backingFormat);
+int virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
+  char *buf,
+  size_t len,
+  int *backingFormat)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+
 virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
 int fd,
 int format);
-- 
1.9.2

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

[libvirt] [PATCH RFC 18/22] storage: Change to new backing store parser

Use the new backing store parser in the backing chain crawler. This
change needs one test change where information about the NBD image are
now parsed differently.
---
 src/storage/storage_driver.c | 90 +---
 tests/virstoragetest.c   | 14 ---
 2 files changed, 9 insertions(+), 95 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cf969c8..ac49553 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3041,56 +3041,6 @@ virStorageFileAccess(virStorageSourcePtr src,
 }


-/**
- * Given a starting point START (a directory containing the original
- * file, if the original file was opened via a relative path; ignored
- * if NAME is absolute), determine the location of the backing file
- * NAME (possibly relative), and compute the relative DIRECTORY
- * (optional) and CANONICAL (mandatory) location of the backing file.
- * Return 0 on success, negative on error.
- */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
-virFindBackingFile(const char *start, const char *path,
-   char **directory, char **canonical)
-{
-/* FIXME - when we eventually allow non-raw network devices, we
- * must ensure that we handle backing files the same way as qemu.
- * For a qcow2 top file of gluster://server/vol/img, qemu treats
- * the relative backing file 'rel' as meaning
- * 'gluster://server/vol/rel', while the backing file '/abs' is
- * used as a local file.  But we cannot canonicalize network
- * devices via canonicalize_file_name(), because they are not part
- * of the local file system.  */
-char *combined = NULL;
-int ret = -1;
-
-if (*path == '/') {
-/* Safe to cast away const */
-combined = (char *)path;
-} else if (virAsprintf(combined, %s/%s, start, path)  0) {
-goto cleanup;
-}
-
-if (directory  !(*directory = mdir_name(combined))) {
-virReportOOMError();
-goto cleanup;
-}
-
-if (!(*canonical = canonicalize_file_name(combined))) {
-virReportSystemError(errno,
- _(Can't canonicalize path '%s'), path);
-goto cleanup;
-}
-
-ret = 0;
-
- cleanup:
-if (combined != path)
-VIR_FREE(combined);
-return ret;
-}
-
-
 /* Recursive workhorse for virStorageFileGetMetadata.  */
 static int
 virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
@@ -3099,7 +3049,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
  virHashTablePtr cycle)
 {
 int ret = -1;
-struct stat st;
 const char *uniqueName;
 char *buf = NULL;
 ssize_t headerLen;
@@ -3151,46 +3100,9 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 goto cleanup;
 }

-if (VIR_ALLOC(backingStore)  0)
+if (!(backingStore = virStorageSourceNewFromBacking(src)))
 goto cleanup;

-if (VIR_STRDUP(backingStore-relPath, src-backingStoreRaw)  0)
-goto cleanup;
-
-if (virStorageIsFile(src-backingStoreRaw)) {
-backingStore-type = VIR_STORAGE_TYPE_FILE;
-
-if (virFindBackingFile(src-relDir,
-   src-backingStoreRaw,
-   backingStore-relDir,
-   backingStore-path)  0) {
-/* the backing file is (currently) unavailable, treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image chains */
-VIR_WARN(Backing file '%s' of image '%s' is missing.,
- src-backingStoreRaw, src-path);
-ret = 0;
-goto cleanup;
-}
-
-/* update the type for local storage */
-if (stat(backingStore-path, st) == 0) {
-if (S_ISDIR(st.st_mode)) {
-backingStore-type = VIR_STORAGE_TYPE_DIR;
-backingStore-format = VIR_STORAGE_FILE_DIR;
-} else if (S_ISBLK(st.st_mode)) {
-backingStore-type = VIR_STORAGE_TYPE_BLOCK;
-}
-}
-} else {
-/* TODO: To satisfy the test case, copy the network URI as path. This
- * will be removed later. */
-backingStore-type = VIR_STORAGE_TYPE_NETWORK;
-
-if (VIR_STRDUP(backingStore-path, src-backingStoreRaw)  0)
-goto cleanup;
-}
-
 if (backingFormat == VIR_STORAGE_FILE_AUTO  !allow_probe)
 backingStore-format = VIR_STORAGE_FILE_RAW;
 else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 746bf63..29297ef 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -730,20 +730,22 @@ mymain(void)
 /* Rewrite qcow2 to use an nbd: protocol as backend */
 virCommandFree(cmd);
 cmd = virCommandNewArgList(qemuimg, rebase, -u, -f, qcow2,
-   -F, raw, -b,

[libvirt] [PATCH RFC 03/22] storage: Rework debugging of storage file access through storage driver

Print the debug statements of individual file access functions from the
main API functions instead of the individual backend functions.

Also enhance initialization debug messages on a per-backend basis.
---
 src/storage/storage_backend_fs.c  | 43 ++-
 src/storage/storage_backend_gluster.c | 30 +---
 src/storage/storage_driver.c  | 27 +++---
 3 files changed, 62 insertions(+), 38 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de27890..3d088ba 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1339,18 +1339,31 @@ virStorageBackend virStorageBackendNetFileSystem = {
 };


+static void
+virStorageFileBackendFileDeinit(virStorageSourcePtr src)
+{
+VIR_DEBUG(deinitializing FS storage file %p (%s:%s), src,
+  virStorageTypeToString(virStorageSourceGetActualType(src)),
+  src-path);
+
+}
+
+
 static int
-virStorageFileBackendFileUnlink(virStorageSourcePtr src)
+virStorageFileBackendFileInit(virStorageSourcePtr src)
 {
-int ret;
+VIR_DEBUG(initializing FS storage file %p (%s:%s), src,
+  virStorageTypeToString(virStorageSourceGetActualType(src)),
+  src-path);

-ret = unlink(src-path);
-/* preserve errno */
+return 0;
+}

-VIR_DEBUG(removing storage file %p(%s): ret=%d, errno=%d,
-  src, src-path, ret, errno);

-return ret;
+static int
+virStorageFileBackendFileUnlink(virStorageSourcePtr src)
+{
+return unlink(src-path);
 }


@@ -1358,21 +1371,16 @@ static int
 virStorageFileBackendFileStat(virStorageSourcePtr src,
   struct stat *st)
 {
-int ret;
-
-ret = stat(src-path, st);
-/* preserve errno */
-
-VIR_DEBUG(stat of storage file %p(%s): ret=%d, errno=%d,
-  src, src-path, ret, errno);
-
-return ret;
+return stat(src-path, st);
 }


 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
 };
@@ -1381,6 +1389,9 @@ virStorageFileBackend virStorageFileBackendFile = {
 virStorageFileBackend virStorageFileBackendBlock = {
 .type = VIR_STORAGE_TYPE_BLOCK,

+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
 .storageFileStat = virStorageFileBackendFileStat,
 };

diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index 8679d96..e578e73 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -539,10 +539,12 @@ struct _virStorageFileBackendGlusterPriv {
 static void
 virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
 {
-VIR_DEBUG(deinitializing gluster storage file %p(%s/%s),
-  src, src-hosts[0].name, src-path);
 virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;

+VIR_DEBUG(deinitializing gluster storage file %p (gluster://%s:%s/%s%s),
+  src, src-hosts-name, src-hosts-port ? src-hosts-port : 0,
+  src-volume, src-path);
+
 if (priv-vol)
 glfs_fini(priv-vol);

@@ -558,12 +560,14 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
 const char *hostname = host-name;
 int port = 0;

-VIR_DEBUG(initializing gluster storage file %p(%s/%s),
-  src, hostname, src-path);
+VIR_DEBUG(initializing gluster storage file %p (gluster://%s:%s/%s%s),
+  src, hostname, host-port ? host-port : 0,
+  NULLSTR(src-volume), src-path);

 if (!src-volume) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(missing gluster volume name for path '%s'), 
src-path);
+   _(missing gluster volume name for path '%s'),
+   src-path);
 return -1;
 }

@@ -619,14 +623,8 @@ static int
 virStorageFileBackendGlusterUnlink(virStorageSourcePtr src)
 {
 virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
-int ret;
-
-ret = glfs_unlink(priv-vol, src-path);
-/* preserve errno */

-VIR_DEBUG(removing storage file %p(%s/%s): ret=%d, errno=%d,
-  src, src-hosts[0].name, src-path, ret, errno);
-return ret;
+return glfs_unlink(priv-vol, src-path);
 }


@@ -635,14 +633,8 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
  struct stat *st)
 {
 virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
-int ret;

-ret = glfs_stat(priv-vol, src-path, st);
-/* preserve errno */
-
-VIR_DEBUG(stat of storage file %p(%s/%s): ret=%d, errno=%d,
-  src, src-hosts[0].name, src-path, ret, errno);
-return ret;
+return

[libvirt] [PATCH RFC 20/22] qemu: Add support for networked disks for block pull/block rebase

Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
 src/qemu/qemu_driver.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b0a79e4..e9cf5d5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14901,6 +14901,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 virDomainDiskDefPtr disk;
 virStorageSourcePtr baseSource = NULL;
 unsigned int baseIndex = 0;
+char *basePath = NULL;

 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -14969,9 +14970,16 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
   base, baseIndex, NULL
 goto endjob;

+if (baseIndex) {
+if (qemuGetDriveSourceString(baseSource, NULL, basePath)  0)
+goto endjob;
+} else {
+if (VIR_STRDUP(basePath, base)  0)
+goto endjob;
+}
+
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device,
-  baseIndex ? baseSource-path : base,
+ret = qemuMonitorBlockJob(priv-mon, device, basePath,
   bandwidth, info, mode, async);
 qemuDomainObjExitMonitor(driver, vm);
 if (ret  0)
@@ -15042,6 +15050,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 }

  cleanup:
+VIR_FREE(basePath);
 VIR_FREE(device);
 if (vm)
 virObjectUnlock(vm);
-- 
1.9.2

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

Re: [libvirt] [Qemu-devel] qemu leaving unix sockets behind after VM is shut down

2014-05-06 Thread Stefan Hajnoczi

On Tue, Apr 01, 2014 at 02:34:58PM -0600, Chris Friesen wrote:
 When running qemu with something like this
 
 -device virtio-serial \
 -chardev socket,path=/tmp/foo,server,nowait,id=foo \
 -device virtserialport,chardev=foo,name=host.port.0
 
 the VM starts up as expected and creates a socket at /tmp/foo as expected.
 
 However, when I shut down the VM the socket at /tmp/foo is left
 behind in the filesystem.  Basically qemu has leaked a file.
 
 With something like OpenStack where we could be creating/destroying
 many VMs this could end up creating a significant number of files in
 the specified directory.
 
 Has any thought been given to either automatically cleaning up the
 unix socket in the filesystem when qemu exits, or else supporting
 the abstract namespace for unix sockets to allow for automatic
 cleanup?

Libvirt has a special case for the monitor socket in its
qemuProcessStop() function.

Are you using the OpenStack libvirt driver?

Perhaps QEMU should support cleanup but first I think we should check
the situation with libvirt.

Stefan

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

Re: [libvirt] [Qemu-devel] qemu leaving unix sockets behind after VM is shut down

On Tue, May 06, 2014 at 03:39:57PM +0200, Stefan Hajnoczi wrote:
 On Tue, Apr 01, 2014 at 02:34:58PM -0600, Chris Friesen wrote:
  When running qemu with something like this
  
  -device virtio-serial \
  -chardev socket,path=/tmp/foo,server,nowait,id=foo \
  -device virtserialport,chardev=foo,name=host.port.0
  
  the VM starts up as expected and creates a socket at /tmp/foo as expected.
  
  However, when I shut down the VM the socket at /tmp/foo is left
  behind in the filesystem.  Basically qemu has leaked a file.
  
  With something like OpenStack where we could be creating/destroying
  many VMs this could end up creating a significant number of files in
  the specified directory.
  
  Has any thought been given to either automatically cleaning up the
  unix socket in the filesystem when qemu exits, or else supporting
  the abstract namespace for unix sockets to allow for automatic
  cleanup?
 
 Libvirt has a special case for the monitor socket in its
 qemuProcessStop() function.
 
 Are you using the OpenStack libvirt driver?
 
 Perhaps QEMU should support cleanup but first I think we should check
 the situation with libvirt.

We have explicit cleanup (ie unlink() call) for the monitor
UNIX socket, but guess we forgot todo the same for other
char device sockets.

We can't rely on QEMU to cleanup since it can obviously
SEGV at any time which would prevent it cleaning up after
itself. So libvirt must have any neccessary cleanup itself,
even if QEMU did make an effort to clean up.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [RFC PATCH] Add support for invtsc timer

On 05/06/14 15:27, Ján Tomko wrote:
 Not yet merged in upstream QEMU:
 https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
 
 Add support for invariant TSC timer running at constant rate in
 all ACPI P-, C- and T-states.
 
 It can be enabled by specifying:
 clock
   timer name='invtsc' present='yes'/
 /clock
 in the domain XML.
 
 Migration and saving the domain does not work with this timer.
 
 The support for this timer is indicated by bit 8 of EDX after calling
 CPUID with 0x8007. It does not show up in /proc/cpuinfo [1]
 and since we're calling qemu without 'enforce', it doesn't error
 out if the host doesn't support this.
 
 Alternatively, we could expose it in libvirt as a cpu flag:
 cpu mode='custom' match='exact'
   model fallback='forbid'qemu64/model
   feature policy='require' name='invtsc'/
 /cpu
 or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is
 requested?
 
 [1]:
 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/powerflags.c?id=30321c7b#n18
 ---
  docs/formatdomain.html.in  |  9 ++--
  docs/schemas/domaincommon.rng  |  1 +
  src/conf/domain_conf.c |  6 +++--
  src/conf/domain_conf.h |  1 +
  src/qemu/qemu_command.c|  6 +
  src/qemu/qemu_migration.c  | 14 
  .../qemuxml2argv-clock-timer-inv-tsc.args  |  5 +
  .../qemuxml2argv-clock-timer-inv-tsc.xml   | 26 
 ++
  tests/qemuxml2argvtest.c   |  1 +
  tests/qemuxml2xmltest.c|  1 +
  10 files changed, 66 insertions(+), 4 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-clock-timer-inv-tsc.xml

...

 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 4249ed5..5154826 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -918,6 +918,7 @@
  choice
valuekvmclock/value
valuehypervclock/value
 +  valueinvtsc/value

I'd prefer to change the name of the feature to invarianttsc in
libvirt's representation, but that's just bikeshedding.

  /choice
/attribute
  /group

...

 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index a9f7fea..c1ffc0f 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -1513,6 +1513,20 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, 
 virDomainObjPtr vm,
  return false;
  }
  
 +for (i = 0; i  def-clock.ntimers; i++) {
 +virDomainTimerDefPtr timer = def-clock.timers[i];
 +
 +if (timer-present != 1)
 +continue;
 +
 +if (timer-name == VIR_DOMAIN_TIMER_NAME_INVTSC) {
 +virReportError(VIR_ERR_OPERATION_INVALID,
 +   _(domain has '%s' timer),
 +   virDomainTimerNameTypeToString(timer-name));
 +return false;
 +}
 +}
 +
  return true;
  }
  

It's a shame that this doesn't work across migration in a way HyperV has
designed it.

ACK once the qemu functionality will be released

Peter




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

Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

2014-05-06 Thread Eduardo Habkost

On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
 On Fri, 2 May 2014 11:43:05 -0300
 Eduardo Habkost ehabk...@redhat.com wrote:
 
  On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
   On Wed, 30 Apr 2014 17:29:28 -0300
   Eduardo Habkost ehabk...@redhat.com wrote:
   
This series allows management code to use object-add on X86CPU 
subclasses, so it
   Is there any reason why device-add couldn't be used?
  
  It needs to work with -machine none, device_add requires a bus to
  exist, and there is no icc-bus on machine_none.
 The thing is that CPUID is a function of machine so using
 -machine none will provide only approximately accurate data.
 I'm not sure that retrieved possibly not accurate data are useful
 for libvirt.

-cpu host doesn't depend on machine, and is the most important thing
right now (because libvirt's checks for host QEMU/kernel/CPU
capabilities is completely broken).

About machine-type data: currently libvirt has no concept of
per-machine-type CPU model data, anyway. We (and libvirt) will need to
address this eventually, but considering our track record, I bet the
QEMU community will take a few years to finally provide that info to
libvirt.

In the meantime, we have a partial solution that fits the current
libvirt data model and is better than the current state (where libvirt
has to duplicate the CPU model data).

Maybe they will use this only for host, maybe they will use this for
all the other CPU models, we are just providing the mechanism, it's
their decision to use it or not.


 
  
  The first thing I considered was making icc-bus user-creatable. Then I
  noticed it wouldn't work because object-add always add objects to
  /objects, not inside the qdev hierarchy (that's where device_add looks
  for the bus).
  
  So, allowing device_add could be possible, but would require changing
  more basic infrastructure: either allowing bus-less devices on
  device_add, or allowing device_add to add devices outside the qdev
  hierarchy, or allowing object-add to create objects outside /objects.
  
  Simply making CPU objects work with object-add was much simpler and less
  intrusive. And it had the interesting side-effect of _not_ doing things
  that are not required for CPU model probing (like creating an actual
  VCPU thread).
  
   
   
can use it to probe for CPU model information without re-running QEMU. 
The main
use case for this is to allow management code to create CPU objects and 
query
the feature-words and filtered-features properties on the new 
objects, to
find out which features each CPU model needs, and to do the same using 
the
host CPU model to check which features can be enabled in a given host.

There's experimental libvirt code to use the new command at:
https://github.com/ehabkost/libvirt/tree/work/cpu-feature-word-query
The experimental code just create the CPU objects to query for feature
information, but doesn't do anything with that data.

Eduardo Habkost (5):
  cpu: Initialize cpu-stopped=true earlier
  cpu: Don't try to pause CPUs if they are already stopped
  pc: Don't crash on apic_accept_pic_intr() if CPU has no apic_state
  target-i386: Make CPU objects user-creatable
  target-i386: Report QOM class name for CPU definitions

 cpus.c| 13 ++---
 exec.c|  1 +
 hw/i386/pc.c  |  2 +-
 qapi-schema.json  |  6 +-
 target-i386/cpu.c |  7 +++
 5 files changed, 24 insertions(+), 5 deletions(-)

   
  
 

-- 
Eduardo

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

Re: [libvirt] [RFC PATCH] Add support for invtsc timer

On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
 Not yet merged in upstream QEMU:
 https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html
 
 Add support for invariant TSC timer running at constant rate in
 all ACPI P-, C- and T-states.

What does this do at the hardware level ? This doesn't seem to be
creating a second TSC timer source in the guest, rather it is
setting a property related to the existing TSC timer. So I think
it might make more sense as an attribute for timer name='tsc'
element instead.

 
 It can be enabled by specifying:
 clock
   timer name='invtsc' present='yes'/
 /clock
 in the domain XML.
 
 Migration and saving the domain does not work with this timer.

Why is that ? The QEMU patch doesn't mention this restriction.

 
 The support for this timer is indicated by bit 8 of EDX after calling
 CPUID with 0x8007. It does not show up in /proc/cpuinfo [1]
 and since we're calling qemu without 'enforce', it doesn't error
 out if the host doesn't support this.

Maybe I'm mis-interpreting the kernel source, but my reading of
that was that this *does* show up in /proc/cpuinfo, but it is
given the name 'constant_tsc' instead of 'invtsc'.

On my host I see 'constant_tsc' and 'nonstop_tsc' in /proc/cpuinfo

 Alternatively, we could expose it in libvirt as a cpu flag:
 cpu mode='custom' match='exact'
   model fallback='forbid'qemu64/model
   feature policy='require' name='invtsc'/
 /cpu
 or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is
 requested?

Yep, I could see that as a valid option. If it is visible
in /proc/cpuinfo, then I think that's a compelling reason for
libvirt to model it as a CPU flag too, rather than pretend it
is a new type of timer when it is just an attribute of the
base TSC timer.

 [1]:
 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/powerflags.c?id=30321c7b#n18

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [Qemu-devel] qemu leaving unix sockets behind after VM is shut down

2014-05-06 Thread Chris Friesen


On 05/06/2014 07:39 AM, Stefan Hajnoczi wrote:

On Tue, Apr 01, 2014 at 02:34:58PM -0600, Chris Friesen wrote:

When running qemu with something like this

-device virtio-serial \
-chardev socket,path=/tmp/foo,server,nowait,id=foo \
-device virtserialport,chardev=foo,name=host.port.0

the VM starts up as expected and creates a socket at /tmp/foo as expected.

However, when I shut down the VM the socket at /tmp/foo is left
behind in the filesystem.  Basically qemu has leaked a file.

With something like OpenStack where we could be creating/destroying
many VMs this could end up creating a significant number of files in
the specified directory.

Has any thought been given to either automatically cleaning up the
unix socket in the filesystem when qemu exits, or else supporting
the abstract namespace for unix sockets to allow for automatic
cleanup?


Libvirt has a special case for the monitor socket in its
qemuProcessStop() function.

Are you using the OpenStack libvirt driver?

Perhaps QEMU should support cleanup but first I think we should check
the situation with libvirt.


Yes, I am in fact using OpenStack/libvirt, and did eventually track down 
libvirt as the code that was cleaning up the monitor socket.


Even so, I think this sort of change would be valid in qemu itself. qemu 
created the files, so really it should be up to qemu to delete them when 
it's done with them.


They're not usable for anything with qemu not running, so there's no 
good reason to leave them laying around.


Chris



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

Re: [libvirt] [PATCH] docs: update README-hacking

2014-05-06 Thread Nehal J Wani

On Tue, May 6, 2014 at 12:32 PM, Chen Hanxiao
chenhanx...@cn.fujitsu.com wrote:
 We didn't have README-valgrind file now.
 So remove related description.

s/didn't/don't
or maybe this is better:
Remove reference of non-existing file 'README-valgrind'


 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  README-hacking | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/README-hacking b/README-hacking
 index 80b022c..4e02fd8 100644
 --- a/README-hacking
 +++ b/README-hacking
 @@ -15,7 +15,7 @@ Specific development tools and versions will be checked for 
 and listed by
  the bootstrap script.

  Valgrind http://valgrind.org/ is also highly recommended, if
 -Valgrind supports your architecture. See also README-valgrind.
 +Valgrind supports your architecture.

  While building from a just-cloned source tree may require installing a
  few prerequisites, later, a plain `git pull  make' should be sufficient.
 --
 1.9.0

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

Was there any plan to create this file?

ACK with the commit message changed.

-- 
Nehal J Wani

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

Re: [libvirt] [PATCH] PCI: Introduce new device binding path using pci_dev.driver_override

2014-05-06 Thread Alex Williamson

Hi Greg,

I think Bjorn is waiting for an ack from you on this.  Do you approve of
this approach from a driver-core perspective?  Thanks,

Alex

On Fri, 2014-04-04 at 14:19 -0600, Alex Williamson wrote:
 The driver_override field allows us to specify the driver for a device
 rather than relying on the driver to provide a positive match of the
 device.  This shortcuts the existing process of looking up the vendor
 and device ID, adding them to the driver new_id, binding the device,
 then removing the ID, but it also provides a couple advantages.
 
 First, the above existing process allows the driver to bind to any
 device matching the new_id for the window where it's enabled.  This is
 often not desired, such as the case of trying to bind a single device
 to a meta driver like pci-stub or vfio-pci.  Using driver_override we
 can do this deterministically using:
 
 echo pci-stub  /sys/bus/pci/devices/:03:00.0/driver_override
 echo :03:00.0  /sys/bus/pci/devices/:03:00.0/driver/unbind
 echo :03:00.0  /sys/bus/pci/drivers_probe
 
 Previously we could not invoke drivers_probe after adding a device
 to new_id for a driver as we get non-deterministic behavior whether
 the driver we intend or the standard driver will claim the device.
 Now it becomes a deterministic process, only the driver matching
 driver_override will probe the device.
 
 To return the device to the standard driver, we simply clear the
 driver_override and reprobe the device:
 
 echo  /sys/bus/pci/devices/:03:00.0/preferred_driver
 echo :03:00.0  /sys/bus/pci/devices/:03:00.0/driver/unbind
 echo :03:00.0  /sys/bus/pci/drivers_probe
 
 Another advantage to this approach is that we can specify a driver
 override to force a specific binding or prevent any binding.  For
 instance when an IOMMU group is exposed to userspace through VFIO
 we require that all devices within that group are owned by VFIO.
 However, devices can be hot-added into an IOMMU group, in which case
 we want to prevent the device from binding to any driver (preferred
 driver = none) or perhaps have it automatically bind to vfio-pci.
 With driver_override it's a simple matter for this field to be set
 internally when the device is first discovered to prevent driver
 matches.
 
 Signed-off-by: Alex Williamson alex.william...@redhat.com
 ---
 
 Changes since RFC:
  - Add ABI documentation (gregkh)
  - Documentation wording clarification (Christoffer)
 
 Nobody puked on the RFC and platform folks have posted a working
 version of this for platform devices, so I guess the only thing left
 to do is formally propose this as a new driver binding mechanism.  I
 don't see much incentive to push this into the driver core since the
 match ultimately needs to be done by the bus driver.  I think this is
 therefore like new_id/remove_id where PCI and USB implement separate,
 but consistent interfaces.
 
 I've pruned the CC list a bit from the RFC, but I've added libvirt
 folks since I expect they would be the first userspace tool that would
 adopt this.  Thanks,
 
 Alex
 
  Documentation/ABI/testing/sysfs-bus-pci |   21 
  drivers/pci/pci-driver.c|   25 +--
  drivers/pci/pci-sysfs.c |   40 
 +++
  include/linux/pci.h |1 +
  4 files changed, 84 insertions(+), 3 deletions(-)
 
 diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
 b/Documentation/ABI/testing/sysfs-bus-pci
 index a3c5a66..898ddc4 100644
 --- a/Documentation/ABI/testing/sysfs-bus-pci
 +++ b/Documentation/ABI/testing/sysfs-bus-pci
 @@ -250,3 +250,24 @@ Description:
   valid.  For example, writing a 2 to this file when sriov_numvfs
   is not 0 and not 2 already will return an error. Writing a 10
   when the value of sriov_totalvfs is 8 will return an error.
 +
 +What:/sys/bus/pci/devices/.../driver_override
 +Date:April 2014
 +Contact: Alex Williamson alex.william...@redhat.com
 +Description:
 + This file allows the driver for a device to be specified which
 + will override standard static and dynamic ID matching.  When
 + specified, only a driver with a name matching the value written
 + to driver_override will have an opportunity to bind to the
 + device.  The override is specified by writing a string to the
 + driver_override file (echo pci-stub  driver_override) and
 + may be cleared with an empty string (echo  driver_override).
 + This returns the device to standard matching rules binding.
 + Writing to driver_override does not automatically unbind the
 + device from its current driver or make any attempt to
 + automatically load the specified driver.  If no driver with a
 + matching name is currently loaded in the kernel, the device
 + will not bind to any

Re: [libvirt] [Qemu-devel] qemu leaving unix sockets behind after VM is shut down

On 05/06/2014 08:52 AM, Chris Friesen wrote:
 Yes, I am in fact using OpenStack/libvirt, and did eventually track down
 libvirt as the code that was cleaning up the monitor socket.
 
 Even so, I think this sort of change would be valid in qemu itself. qemu
 created the files, so really it should be up to qemu to delete them when
 it's done with them.
 
 They're not usable for anything with qemu not running, so there's no
 good reason to leave them laying around.

In the case of sVirt with SELinux labeling, it's often the case that
libvirt has to pre-create the file (as qemu itself is prevented from
creating the file itself) - that is also an argument for libvirt doing
the cleanup.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH v5] Add helper program to create custom leases

2014-05-06 Thread Nehal J Wani

Ping!

On Sat, Apr 26, 2014 at 5:29 AM, Nehal J Wani nehaljw.k...@gmail.com wrote:
 Introduce helper program to catch events from dnsmasq and maintain a custom
 lease file per network. It supports dhcpv4 and dhcpv6. The file is saved as
 interface-name.status.

 Each lease contains the following info:
 expiry-time (epoch time) mac iaid ip-address hostname clientid

 Example of custom leases file content:
 [
 {
 iaid: 1221229,
 ip-address: 2001:db8:ca2:2:1::95,
 mac-address: 52:54:00:12:a2:6d,
 hostname: Fedora20,
 client-id: 00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55,
 expiry-time: 1393244216
 },
 {
 ip-address: 192.168.150.208,
 mac-address: 52:54:00:11:56:b3,
 hostname: Wani-PC,
 client-id: 01:52:54:00:11:56:b3,
 expiry-time: 1393244248
 }
 ]

 src/Makefile.am:
* Add options to compile the helper program

 src/network/bridge_driver.c:
* Introduce networkDnsmasqLeaseFileNameCustom()
* Invoke helper program along with dnsmasq
* Delete the .status file when corresponding n/w is destroyed.

 src/network/leaseshelper.c
* Helper program to create the custom lease file

 ---
  v5:
  * More comments added, for better explanation
  * Use of virFileFindResource() to identify proper path to helper binary
  * Use of VIR_ENUM_IMPL for handling action events added

  v4:
  * Addition of pidfile and a corresponding lock for it
  * Make correction for dnsmasq  2.52 (Only IPv4)
  * Move helper file from src/util to src/network
  * Increase limit on max size of leases file
  * Refer: https://www.redhat.com/archives/libvir-list/2014-March/msg01038.html

  v3:
  * Improved file handling, removed redundant copying, introduced --help and 
 --version
  * Refer: 
 https://www.redhat.com/archives/libvir-list/2014-February/msg01431.html

  v2:
  * Changed format to JSON
  * Refer: 
 https://www.redhat.com/archives/libvir-list/2014-January/msg01234.html

  v1:
  * Refer: 
 https://www.redhat.com/archives/libvir-list/2014-January/msg00626.html

  src/Makefile.am |   22 +++
  src/network/bridge_driver.c |   27 
  src/network/leaseshelper.c  |  360 
 +++
  3 files changed, 409 insertions(+), 0 deletions(-)
  create mode 100644 src/network/leaseshelper.c

 diff --git a/src/Makefile.am b/src/Makefile.am
 index dd0abe7..fbe72c4 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -859,6 +859,9 @@ STORAGE_HELPER_DISK_SOURCES = 
   \
  UTIL_IO_HELPER_SOURCES =   \
 util/iohelper.c

 +NETWORK_LEASES_HELPER_SOURCES = \
 +   network/leaseshelper.c
 +
  # Network filters
  NWFILTER_DRIVER_SOURCES =  \
 nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c   \
 @@ -2466,6 +2469,25 @@ libvirt_iohelper_CFLAGS = \
 $(AM_CFLAGS) \
 $(PIE_CFLAGS) \
 $(NULL)
 +
 +if WITH_NETWORK
 +libexec_PROGRAMS += libvirt_leaseshelper
 +libvirt_leaseshelper_SOURCES = $(NETWORK_LEASES_HELPER_SOURCES)
 +libvirt_leaseshelper_LDADD =   \
 +   libvirt_util.la \
 +   ../gnulib/lib/libgnu.la
 +if WITH_DTRACE_PROBES
 +libvirt_leaseshelper_LDADD += libvirt_probes.lo
 +endif WITH_DTRACE_PROBES
 +
 +libvirt_leaseshelper_CFLAGS = \
 +  $(AM_CFLAGS) \
 +   $(PIE_CFLAGS) \
 +   $(NULL)
 +else ! WITH_NETWORK
 +EXTRA_DIST += $(NETWORK_LEASES_HELPER_SOURCES)
 +endif ! WITH_NETWORK
 +
  endif WITH_LIBVIRTD

  if WITH_STORAGE_DISK
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index 201b22f..ade664d 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -210,6 +210,16 @@ networkDnsmasqLeaseFileNameFunc 
 networkDnsmasqLeaseFileName =
  networkDnsmasqLeaseFileNameDefault;

  static char *
 +networkDnsmasqLeaseFileNameCustom(const char *bridge)
 +{
 +char *leasefile;
 +
 +ignore_value(virAsprintf(leasefile, %s/%s.status,
 + driverState-dnsmasqStateDir, bridge));
 +return leasefile;
 +}
 +
 +static char *
  networkDnsmasqConfigFileName(const char *netname)
  {
  char *conffile;
 @@ -245,6 +255,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver,
virNetworkObjPtr net)
  {
  char *leasefile = NULL;
 +char *customleasefile = NULL;
  char *radvdconfigfile = NULL;
  char *configfile = NULL;
  char *radvdpidbase = NULL;
 @@ -263,6 +274,9 @@ networkRemoveInactive(virNetworkDriverStatePtr driver,
  if (!(leasefile = networkDnsmasqLeaseFileName(def-name)))
  goto cleanup;

 +if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(def-bridge)))
 +goto cleanup;
 +
  if (!(radvdconfigfile =

Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

2014-05-06 Thread Igor Mammedov

On Tue, 6 May 2014 11:42:56 -0300
Eduardo Habkost ehabk...@redhat.com wrote:

 On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
  On Fri, 2 May 2014 11:43:05 -0300
  Eduardo Habkost ehabk...@redhat.com wrote:
  
   On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
On Wed, 30 Apr 2014 17:29:28 -0300
Eduardo Habkost ehabk...@redhat.com wrote:

 This series allows management code to use object-add on X86CPU 
 subclasses, so it
Is there any reason why device-add couldn't be used?
   
   It needs to work with -machine none, device_add requires a bus to
   exist, and there is no icc-bus on machine_none.
  The thing is that CPUID is a function of machine so using
  -machine none will provide only approximately accurate data.
  I'm not sure that retrieved possibly not accurate data are useful
  for libvirt.
 
 -cpu host doesn't depend on machine, and is the most important thing
 right now (because libvirt's checks for host QEMU/kernel/CPU
 capabilities is completely broken).
true, but device-add/-cpu host could be used right now to get the
same CPUID data wile using any machine type or default one without
any of this patches.

 
 About machine-type data: currently libvirt has no concept of
 per-machine-type CPU model data, anyway. We (and libvirt) will need to
 address this eventually, but considering our track record, I bet the
 QEMU community will take a few years to finally provide that info to
 libvirt.
I don't think QEMU is issue here, libvirt can use device-add to
probe accurate CPUID on all CPU models on a given machine type now.
libvirt should be fixed to care about machine type and use it to get
correct CPUID data that QEMU provides.

 
 In the meantime, we have a partial solution that fits the current
 libvirt data model and is better than the current state (where libvirt
 has to duplicate the CPU model data).
Replacing one set of inaccurate CPUIDs with another is hardly solution.

 
 Maybe they will use this only for host, maybe they will use this for
 all the other CPU models, we are just providing the mechanism, it's
 their decision to use it or not.
As I've said above libvirt could use device-add xxx-host or -cpu host
to get it and second point I object to is providing yet another way
to produce inaccurate CPUID info (libvirt has one already) and to do
so hack [patches 1-3] CPU infrastructure to run out of context it's
supposed to run in. While current API already allows to get correct
CPUID data.

IMHO, libvirt side should take advantage of information QEMU already
provides.

 
 
  
   
   The first thing I considered was making icc-bus user-creatable. Then I
   noticed it wouldn't work because object-add always add objects to
   /objects, not inside the qdev hierarchy (that's where device_add looks
   for the bus).
   
   So, allowing device_add could be possible, but would require changing
   more basic infrastructure: either allowing bus-less devices on
   device_add, or allowing device_add to add devices outside the qdev
   hierarchy, or allowing object-add to create objects outside /objects.
   
   Simply making CPU objects work with object-add was much simpler and less
   intrusive. And it had the interesting side-effect of _not_ doing things
   that are not required for CPU model probing (like creating an actual
   VCPU thread).
   


 can use it to probe for CPU model information without re-running 
 QEMU. The main
 use case for this is to allow management code to create CPU objects 
 and query
 the feature-words and filtered-features properties on the new 
 objects, to
 find out which features each CPU model needs, and to do the same 
 using the
 host CPU model to check which features can be enabled in a given 
 host.
 
 There's experimental libvirt code to use the new command at:
 
 https://github.com/ehabkost/libvirt/tree/work/cpu-feature-word-query
 The experimental code just create the CPU objects to query for feature
 information, but doesn't do anything with that data.
 
 Eduardo Habkost (5):
   cpu: Initialize cpu-stopped=true earlier
   cpu: Don't try to pause CPUs if they are already stopped
   pc: Don't crash on apic_accept_pic_intr() if CPU has no apic_state
   target-i386: Make CPU objects user-creatable
   target-i386: Report QOM class name for CPU definitions
 
  cpus.c| 13 ++---
  exec.c|  1 +
  hw/i386/pc.c  |  2 +-
  qapi-schema.json  |  6 +-
  target-i386/cpu.c |  7 +++
  5 files changed, 24 insertions(+), 5 deletions(-)
 

   
  
 
 -- 
 Eduardo
 


-- 
Regards,
  Igor

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

Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

2014-05-06 Thread Eduardo Habkost

On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote:
 On Tue, 6 May 2014 11:42:56 -0300
 Eduardo Habkost ehabk...@redhat.com wrote:
 
  On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
   On Fri, 2 May 2014 11:43:05 -0300
   Eduardo Habkost ehabk...@redhat.com wrote:
   
On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
 On Wed, 30 Apr 2014 17:29:28 -0300
 Eduardo Habkost ehabk...@redhat.com wrote:
 
  This series allows management code to use object-add on X86CPU 
  subclasses, so it
 Is there any reason why device-add couldn't be used?

It needs to work with -machine none, device_add requires a bus to
exist, and there is no icc-bus on machine_none.
   The thing is that CPUID is a function of machine so using
   -machine none will provide only approximately accurate data.
   I'm not sure that retrieved possibly not accurate data are useful
   for libvirt.
  
  -cpu host doesn't depend on machine, and is the most important thing
  right now (because libvirt's checks for host QEMU/kernel/CPU
  capabilities is completely broken).
 true, but device-add/-cpu host could be used right now to get the
 same CPUID data wile using any machine type or default one without
 any of this patches.

device_add can't be used with -machine none.

 
  
  About machine-type data: currently libvirt has no concept of
  per-machine-type CPU model data, anyway. We (and libvirt) will need to
  address this eventually, but considering our track record, I bet the
  QEMU community will take a few years to finally provide that info to
  libvirt.
 I don't think QEMU is issue here, libvirt can use device-add to
 probe accurate CPUID on all CPU models on a given machine type now.
 libvirt should be fixed to care about machine type and use it to get
 correct CPUID data that QEMU provides.

True, it should. But we still need a solution for the -cpu host problem.

 
  
  In the meantime, we have a partial solution that fits the current
  libvirt data model and is better than the current state (where libvirt
  has to duplicate the CPU model data).
 Replacing one set of inaccurate CPUIDs with another is hardly solution.

We could continue arguing about this, but let's ignore the issue about
probing for CPU model information by now, and let's focus on host
capability probing (-cpu host), then.

How do you propose fixing that in a reasonable time (in QEMU 2.1)
without requiring libvirt to re-run QEMU?


 
  
  Maybe they will use this only for host, maybe they will use this for
  all the other CPU models, we are just providing the mechanism, it's
  their decision to use it or not.
 As I've said above libvirt could use device-add xxx-host or -cpu host
 to get it and second point I object to is providing yet another way
 to produce inaccurate CPUID info (libvirt has one already) and to do
 so hack [patches 1-3] CPU infrastructure to run out of context it's
 supposed to run in. While current API already allows to get correct
 CPUID data.
 
 IMHO, libvirt side should take advantage of information QEMU already
 provides.
 

Current API requires re-running QEMU to query the information. This
series allows it to be run with the -machine none QEMU instance that
is already run by libvirt.


  
  
   

The first thing I considered was making icc-bus user-creatable. Then I
noticed it wouldn't work because object-add always add objects to
/objects, not inside the qdev hierarchy (that's where device_add looks
for the bus).

So, allowing device_add could be possible, but would require changing
more basic infrastructure: either allowing bus-less devices on
device_add, or allowing device_add to add devices outside the qdev
hierarchy, or allowing object-add to create objects outside /objects.

Simply making CPU objects work with object-add was much simpler and less
intrusive. And it had the interesting side-effect of _not_ doing things
that are not required for CPU model probing (like creating an actual
VCPU thread).

 
 
  can use it to probe for CPU model information without re-running 
  QEMU. The main
  use case for this is to allow management code to create CPU objects 
  and query
  the feature-words and filtered-features properties on the new 
  objects, to
  find out which features each CPU model needs, and to do the same 
  using the
  host CPU model to check which features can be enabled in a given 
  host.
  
  There's experimental libvirt code to use the new command at:
  
  https://github.com/ehabkost/libvirt/tree/work/cpu-feature-word-query
  The experimental code just create the CPU objects to query for 
  feature
  information, but doesn't do anything with that data.
  
  Eduardo Habkost (5):
cpu: Initialize cpu-stopped=true earlier
cpu: Don't try to pause CPUs if they are already stopped
pc:

Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

2014-05-06 Thread Andreas Färber

Am 06.05.2014 22:19, schrieb Eduardo Habkost:
 On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote:
 On Tue, 6 May 2014 11:42:56 -0300
 Eduardo Habkost ehabk...@redhat.com wrote:
 On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
 On Fri, 2 May 2014 11:43:05 -0300
 Eduardo Habkost ehabk...@redhat.com wrote:
 On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
 On Wed, 30 Apr 2014 17:29:28 -0300
 Eduardo Habkost ehabk...@redhat.com wrote:
 This series allows management code to use object-add on X86CPU 
 subclasses, so it
 Is there any reason why device-add couldn't be used?
 It needs to work with -machine none, device_add requires a bus to
 exist, and there is no icc-bus on machine_none.
 The thing is that CPUID is a function of machine so using
 -machine none will provide only approximately accurate data.
 I'm not sure that retrieved possibly not accurate data are useful
 for libvirt.
 -cpu host doesn't depend on machine, and is the most important thing
 right now (because libvirt's checks for host QEMU/kernel/CPU
 capabilities is completely broken).
 true, but device-add/-cpu host could be used right now to get the
 same CPUID data wile using any machine type or default one without
 any of this patches.
 
 device_add can't be used with -machine none.

I see no reason why we couldn't *make* CPUs work on -machine none. The
ICC bus and bridge were a hack to make APIC(?) hot-add work in face of
SysBus; if that prohibits other valid uses now, then evaluating Igor's
memory work for CPU might be an option.

I'm not aware of any real X86CPU dependency on ICCBus, so we might as
well make -device place it on SysBus if no ICCBus is available...
probably much more invasive and potentially dangerous though.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

Re: [libvirt] [RFC PATCH] Add support for invtsc timer

2014-05-06 Thread Eduardo Habkost

On Tue, May 06, 2014 at 03:50:48PM +0100, Daniel P. Berrange wrote:
On Tue, May 06, 2014 at 03:27:20PM +0200, Ján Tomko wrote:
Not yet merged in upstream QEMU:
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg05024.html

Add support for invariant TSC timer running at constant rate in
all ACPI P-, C- and T-states.

What does this do at the hardware level ? This doesn't seem to be
creating a second TSC timer source in the guest, rather it is
setting a property related to the existing TSC timer. So I think
it might make more sense as an attribute for timer name='tsc'
element instead.

It can be enabled by specifying:
clock
timer name='invtsc' present='yes'/
/clock
in the domain XML.

Migration and saving the domain does not work with this timer.

Why is that ? The QEMU patch doesn't mention this restriction.

See http://marc.info/?l=qemu-develm=139828711719342w=2

The support for this timer is indicated by bit 8 of EDX after calling
CPUID with 0x8007. It does not show up in /proc/cpuinfo [1]
and since we're calling qemu without 'enforce', it doesn't error
out if the host doesn't support this.

libvirt really needs to address this bug (lack of enforce flag). It
was not so serious a few years ago, but now we have lots of features
that depend on support on KVM kernel code. And now we have this feature
that is unlikely to be included in a CPU model by default and likely to
be configured explicitly, so existing CPU level or family/model checks
won't be enough.

QEMU now has the filtered-features X86CPU property that can be used by
libvirt to emulate enforce behavior with appropriate error reporting.

Maybe I'm mis-interpreting the kernel source, but my reading of
that was that this *does* show up in /proc/cpuinfo, but it is
given the name 'constant_tsc' instead of 'invtsc'.

On my host I see 'constant_tsc' and 'nonstop_tsc' in /proc/cpuinfo

invtsc itself doesn't show up on /proc/cpuinfo directly, though.
nonstop_tsc and constant-tsc are Linux-specific CPU capability flags
that are set because invtsc is present. I find it confusing, I don't
know why Linux doesn't simply show the feature directly like it does for
all other feature flag bits.

Alternatively, we could expose it in libvirt as a cpu flag:
cpu mode='custom' match='exact'
model fallback='forbid'qemu64/model
feature policy='require' name='invtsc'/
/cpu
or maybe add +invtsc to qemu args when the 'nonstop_tsc' flag is
requested?

Yep, I could see that as a valid option. If it is visible
in /proc/cpuinfo, then I think that's a compelling reason for
libvirt to model it as a CPU flag too, rather than pretend it
is a new type of timer when it is just an attribute of the
base TSC timer.

Even if the guest doesn't show it on /proc/cpuinfo, it is a CPUID
feature flag, so this approach seems valid to me.

[1]:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/powerflags.c?id=30321c7b#n18

Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--
Eduardo

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

Re: [libvirt] [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

On 05/06/2014 02:19 PM, Eduardo Habkost wrote:

 IMHO, libvirt side should take advantage of information QEMU already
 provides.

 
 Current API requires re-running QEMU to query the information. This
 series allows it to be run with the -machine none QEMU instance that
 is already run by libvirt.

Therein is the reason libvirt isn't using what qemu already has -
spawning multiple qemu instances (one per reported machine type)
multiplied by the number of qemu binaries does not scale well, when
compared to starting a single qemu -machine none, and doing all queries
on that one machine.  So the point of this patch is getting us closer to
the point where libvirt can learn accurate cpu model information for
multiple machines all from a single qemu invocation.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] docs: update README-hacking

On 05/06/2014 09:33 AM, Nehal J Wani wrote:
 On Tue, May 6, 2014 at 12:32 PM, Chen Hanxiao
 chenhanx...@cn.fujitsu.com wrote:
 We didn't have README-valgrind file now.
 So remove related description.
 
 s/didn't/don't
 or maybe this is better:
 Remove reference of non-existing file 'README-valgrind'

I went with:

We don't have a README-valgind file.


 
 Was there any plan to create this file?

No; it's copy-and-paste leftovers from when Jim Meyering copied from GNU
coreutils, where such a file exists, but is in no shape for reuse in
libvirt.

 
 ACK with the commit message changed.

Now pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [RESEND PATCH] Add support for timestamping QEMU logs

On 05/06/2014 07:36 AM, Ján Tomko wrote:
 QEMU commit 5e2ac51 added a boolean '-msg timestamp=[on|off]'
 option, which can enable timestamps on errors:
 $ qemu-system-x86_64 -msg timestamp=on zghhdorf
 2014-04-09T13:25:46.779484Z qemu-system-x86_64: -msg timestamp=on: could
 not open disk image zghhdorf: Could not open 'zghhdorf': No such file or
 directory
 
 Enable this timestamp if the QEMU binary supports it.
 
 Add a 'log_timestamp' option to qemu.conf for disabling this behavior.
 ---
 Rebased to latest master, this time with the test files added.
 
 previous posting of the patch:
 https://www.redhat.com/archives/libvir-list/2014-April/msg00422.html
 
  src/qemu/libvirtd_qemu.aug |  3 ++
  src/qemu/qemu.conf |  8 ++
  src/qemu/qemu_capabilities.c   |  5 
  src/qemu/qemu_capabilities.h   |  1 +
  src/qemu/qemu_command.c|  4 +++
  src/qemu/qemu_conf.c   |  4 +++
  src/qemu/qemu_conf.h   |  2 ++
  src/qemu/test_libvirtd_qemu.aug.in |  1 +
  tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |  1 +
  tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |  1 +
  .../qemuxml2argv-minimal-msg-timestamp.args|  6 
  .../qemuxml2argv-minimal-msg-timestamp.xml | 32 
 ++
  tests/qemuxml2argvtest.c   |  1 +
  13 files changed, 69 insertions(+)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-minimal-msg-timestamp.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-minimal-msg-timestamp.xml

ACK with nit:

 @@ -1146,6 +1147,9 @@ virQEMUCapsComputeCmdFlags(const char *help,
  if (strstr(help, -machine))
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT);
  
 +if (strstr(help, -msg timestamp))
 +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP);
 +

This hunk is dead code.  -msg timestamp was not added until qemu new
enough to support QMP probing, so we shouldn't be scraping -help output
for it.  It can be re-added as a downstream-only patch to anyone (hello
RHEL) that backports the qemu feature without supporting QMP probing, as
needed, but no need to carry it upstream.

  /*
   * Handling of -incoming arg with varying features
   *  -incoming tcp(kvm = 79, qemu = 0.10.0)
 @@ -2378,6 +2382,7 @@ static struct virQEMUCapsCommandLineProps 
 virQEMUCapsCommandLine[] = {
  { boot-opts, strict, QEMU_CAPS_BOOT_STRICT },
  { boot-opts, reboot-timeout, QEMU_CAPS_REBOOT_TIMEOUT },
  { spice, disable-agent-file-xfer, QEMU_CAPS_SPICE_FILE_XFER_DISABLE 
 },
 +{ msg, timestamp, QEMU_CAPS_MSG_TIMESTAMP },

This is sufficient for setting the cap in upstream libvirt.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [libvirt PATCH] udev: consider the device a CDROM when ID_CDROM=1

On 05/06/2014 05:11 AM, Giuseppe Scrivano wrote:
 Daniel P. Berrange berra...@redhat.com writes:
 
 On Wed, Apr 23, 2014 at 12:42:01PM +0200, Giuseppe Scrivano wrote:
 Some CDROM devices are reported by udev to have an ID_TYPE=generic
 thus it is necessary to check if ID_CDROM is present.

 As a side effect, treating ID_TYPE=generic as a missing ID_TYPE will
 enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the
 udevKludgeStorageType heuristic.

 Signed-off-by: Giuseppe Scrivano gscri...@redhat.com
 ---
  src/node_device/node_device_udev.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 ACK
 
 Thanks.
 
 Can somebody please push the patch as I've no write access to the repo?

Done.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [libvirt PATCH] udev: consider the device a CDROM when ID_CDROM=1

On 04/23/2014 04:42 AM, Giuseppe Scrivano wrote:
 Some CDROM devices are reported by udev to have an ID_TYPE=generic
 thus it is necessary to check if ID_CDROM is present.
 
 As a side effect, treating ID_TYPE=generic as a missing ID_TYPE will
 enable checks for ID_DRIVE_FLASH_SD and ID_DRIVE_FLOPPY and the
 udevKludgeStorageType heuristic.
 
 Signed-off-by: Giuseppe Scrivano gscri...@redhat.com
 ---

  if (udevGetStringProperty(device,
ID_TYPE,
 -  data-storage.drive_type) != PROPERTY_FOUND) {
 +  data-storage.drive_type) != PROPERTY_FOUND
 +|| STREQ(def-caps-data.storage.drive_type, generic)) {

Style: HACKING documents that we tend to leave || hanging on the first
line, not wrapped to the next line (closer to kernel style than GNU style).

  int tmp_int = 0;
  
  /* All floppy drives have the ID_DRIVE_FLOPPY prop. This is
 @@ -1104,6 +1105,12 @@ static int udevProcessStorage(struct udev_device 
 *device,
  
  if (VIR_STRDUP(data-storage.drive_type, floppy)  0)
  goto out;
 +} else if ((udevGetIntProperty(device, ID_CDROM,
 +tmp_int, 0) == PROPERTY_FOUND) 
 +(tmp_int == 1)) {

Style: over-parenthesized, and indentation is odd.

 +
 +if (VIR_STRDUP(data-storage.drive_type, cd)  0)
 +goto out;
  } else if ((udevGetIntProperty(device, ID_DRIVE_FLASH_SD,
 tmp_int, 0) == PROPERTY_FOUND) 
 (tmp_int == 1)) {

But you were just copying existing oddness.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [libvirt-users] Virsh Attach Volume Fails

On 05/06/2014 02:51 AM, Vilobh Meshram wrote:
 Hi,
 
 When trying to attach a created volume to an instance using the virsh utility 
 I am running into this error :-
 
 
 [] $ sudo virsh attach-device instance-0015 disk.xml
 
 error: Failed to attach device from disk.xml
 
 error: internal error unable to execute QEMU command 
 '__com.redhat_drive_add': Device 'drive-virtio-disk21' could not be 
 initialized

That's an error message from RHEL qemu, and not available from upstream
qemu nor triggered by upstream libvirt (that is, a RHEL-specific
downstream patch to libvirt is attempting to use a downstream qemu
command that upstream does not have).  Have you opened a support ticket
with Red Hat?

 
 
 For experimental purpose I have libvirt 0.10.2 and QEMU 0.12.1. The volume to 
 be attached is on a remote storage.

This list is probably better suited to help if you can reproduce with
upstream versions (libvirt 1.2.4 and qemu 2.0), or at a bare minimum,
post the contents of disk.xml that is failing on you.  It's hard to say
whether the error is in libvirt (perhaps botched SELinux permissions
getting in the way?) or in qemu; but since libvirt is just parroting
qemu's error message, the investigation needs to look more at why qemu
is throwing the message.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH v7 1/6] Introduce virDomainFSFreeze() and virDomainFSThaw() public API

On 05/02/2014 03:48 AM, Daniel P. Berrange wrote:
 On Thu, May 01, 2014 at 08:05:48PM -0400, Tomoki Sekiyama wrote:
 These will freeze and thaw filesystems within guest specified by
 @mountpoints parameters. The parameters can be NULL and 0, then the all

s/the all/all/

 mounted filesystems are frozen or thawed. @flags parameter, which are
 currently not used, is for future extensions.

 Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@hds.com
 ---
  include/libvirt/libvirt.h.in |   10 +
  src/driver.h |   14 ++
  src/libvirt.c|   93 
 ++
  src/libvirt_public.syms  |6 +++
  4 files changed, 123 insertions(+)

 
 + * may be required depending on hypervisor used). If @mountpoints is NULL or
 + * @nmountpoints is 0, every mounted filesystem on the guest is frozen.

s/or/and/ (since the code requires both or neither to be 0)

 + * support mountpoints argument), @mountpoints may be ignored.

s/be ignored/need to be NULL/

 
 ACK

Pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH RFC 01/22] qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage

On 05/06/2014 07:36 AM, Peter Krempa wrote:
 Refactor the function to accept a virStorageSourcePtr instead of just
 the path, add a check to run it only on local storage and fix callers
 (possibly by using a newly introduced wrapper that wraps a path in the
  virStorageSource struct for legacy code)
 ---
  src/qemu/qemu_driver.c | 77 
 --
  1 file changed, 49 insertions(+), 28 deletions(-)
 

  {
  /* The easiest way to label a single file with the same
   * permissions it would have as if part of the disk chain is to
   * temporarily modify the disk in place.  */

Actually, the easiest way is probably to modify the audit and security
code to operate on a virStorageSourcePtr rather than a
virDomainDiskDefPtr - but that's refactoring that can be done independently.

 -char *origsrc = disk-src.path;
 -int origformat = disk-src.format;
 -virStorageSourcePtr origchain = disk-src.backingStore;
 +virStorageSource origdisk;
  bool origreadonly = disk-readonly;
 +virQEMUDriverConfigPtr cfg = NULL;
  int ret = -1;
 -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
 -disk-src.path = (char *) file; /* casting away const is safe here */
 -disk-src.format = VIR_STORAGE_FILE_RAW;
 -disk-src.backingStore = NULL;
 +/* XXX Labelling of non-local files isn't currently supported */
 +if (virStorageSourceGetActualType(disk-src) == 
 VIR_STORAGE_TYPE_NETWORK)
 +return 0;
 +
 +cfg = virQEMUDriverGetConfig(driver);
 +
 +/* XXX This will be easier when disk-src will be a pointer */

will be as in later patches make the conversion? Might read better as
would be easier if disk-were a pointer

 +memcpy(origdisk, disk-src, sizeof(origdisk));
 +memcpy(disk-src, elem, sizeof(*elem));

And indeed, doing a pass over the source tree to use a storageSource via
pointer rather than inline struct would be a fairly mechanical (but
probably large) patch worth doing.

 @@ -12061,6 +12063,25 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr 
 driver,
 
 
  static int
 +qemuDomainPrepareDiskChainElementPath(virQEMUDriverPtr driver,
 +  virDomainObjPtr vm,
 +  virDomainDiskDefPtr disk,
 +  const char *file,
 +  qemuDomainDiskChainMode mode)
 +{
 +virStorageSource src;
 +
 +memset(src, 0, sizeof(src));
 +
 +src.type = VIR_STORAGE_TYPE_FILE;
 +src.format = VIR_STORAGE_FILE_RAW;
 +src.path = (char *) file; /* casting away const is safe here */

Indeed; we can later add a VIR_STRDUP and VIR_FREE if the internals make
it no longer safe.

ACK with comment cleanup.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCH 2/2] storage: Resolve issues in failure path

2014-05-06 Thread John Ferlan

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

Refactoring in commit id '0c2305b3' resulted in the wrong storage
volume object being passed to the new storageVolDeleteInternal().
It should have passed 'voldef' which is the address found in the
pool-volumes.objs[i] array.  By passing 'voldef', the DeleteInternal
code will find and remove the voldef from the volumes.objs[] list.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/storage/storage_driver.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 789744f..8299824 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1766,12 +1766,11 @@ storageVolCreateXML(virStoragePoolPtr obj,
 voldef-building = 0;
 pool-asyncjobs--;
 
-voldef = NULL;
-
 if (buildret  0) {
-storageVolDeleteInternal(volobj, backend, pool, buildvoldef,
+VIR_FREE(buildvoldef);
+storageVolDeleteInternal(volobj, backend, pool, voldef,
  0, false);
-buildvoldef = NULL;
+voldef = NULL;
 goto cleanup;
 }
 
-- 
1.9.0

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

[libvirt] [PATCH 0/2] Fix bug on buildVol error path

2014-05-06 Thread John Ferlan

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

Changes as a result of commit id '0c2305b3', see:

http://www.redhat.com/archives/libvir-list/2014-April/msg00375.html

Resulted in a regression when there was a failure to buildVol the 
volume wasn't removed from the storage driver list because the wrong
local storage pointer was passed to the DeleteInternal routine.

Additionally a preexisting condition where if deleteVol didn't exist
or failed, the volume would not be removed from volumes.objs[] even
though it doesn't exist.  This results in virsh vol-list and vol-info
not finding the volume and emitting error messages.


John Ferlan (2):
  storage: Need to ensure removal of voldef from driver list
  storage: Resolve issues in failure path

 src/storage/storage_driver.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

-- 
1.9.0

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

[libvirt] [PATCH 1/2] storage: Need to ensure removal of voldef from driver list

2014-05-06 Thread John Ferlan

If for some reason a volume cannot be built in the pool (eg, the
buildVol call fails), then the error code may or may not properly
remove the volume from the pool-volumes.objs[] list.

Removal from the list is dependent on the 'deleteVol()' being present
in the backend *and* calling it returning success. If either failed,
then the volobj would never be removed from the list until a pool
refresh was done (or libvirtd restarted).

Note: This is true prior to commit id '0c2305b3' as well where
the cleanup would have been called prior to removal, but removal
never occurred due to various goto statements.

Since the volume would have been added to the list prior to the
attempt to build the volume, modify the storageVolDeleteInternal()
to perform removal during cleanup.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/storage/storage_driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 542b382..789744f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1581,6 +1581,9 @@ storageVolDeleteInternal(virStorageVolPtr obj,
 pool-def-available += vol-target.allocation;
 }
 
+ret = 0;
+
+ cleanup:
 for (i = 0; i  pool-volumes.count; i++) {
 if (pool-volumes.objs[i] == vol) {
 VIR_INFO(Deleting volume '%s' from storage pool '%s',
@@ -1591,9 +1594,6 @@ storageVolDeleteInternal(virStorageVolPtr obj,
 break;
 }
 }
-ret = 0;
-
- cleanup:
 return ret;
 }
 
-- 
1.9.0

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

Re: [libvirt] [PATCH v7 2/6] remote: Implement virDomainFSFreeze and virDomainFSThaw

On 05/01/2014 06:05 PM, Tomoki Sekiyama wrote:
 New rules are added in fixup_name in gendispatch.pl to keep the name
 FSFreeze and FSThaw. This adds a new ACL permission 'fs_freeze',
 which is also applied to VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag.
 
 Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@hds.com
 Acked-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/access/viraccessperm.c   |2 +-
  src/access/viraccessperm.h   |6 ++
  src/remote/remote_driver.c   |2 ++
  src/remote/remote_protocol.x |   38 --
  src/remote_protocol-structs  |   18 ++

'make check' says you missed some pieces here:

--- remote_protocol-structs 2014-05-06 17:53:59.624852386 -0600
+++ remote_protocol-struct-t3   2014-05-06 17:55:10.878391439 -0600
@@ -2435,6 +2435,9 @@
 } mountpoints;
 u_int  flags;
 };
+struct remote_domain_fsfreeze_ret {
+intfilesystems;
+};
 struct remote_domain_fsthaw_args {
 remote_nonnull_domain  dom;
 struct {
@@ -2443,6 +2446,9 @@
 } mountpoints;
 u_int  flags;
 };
+struct remote_domain_fsthaw_ret {
+intfilesystems;
+};
 enum remote_procedure {
 REMOTE_PROC_CONNECT_OPEN = 1,
 REMOTE_PROC_CONNECT_CLOSE = 2,


 +/**
 + * @desc: Freeze and thaw domain filesystems
 + * @message: Freezing and thawing domain filesystems require 
 authorization

s/require/requires/

Fixed and pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [question]qemu: migrate and destory vm immediately on destination causes libvirt deadlock or segment fault

2014-05-06 Thread Wangrui (K)

Hi everyone,

I do ram migration operation in KVM environment(libvirt1.2.4 qemu1.5.1).
I encountered libvirtd deadlock or segmentfault when I destroy the
migration VM on destination.
I got the problem by flowing steps:
step 1: migrate VM.
step 2: execute virsh destroy [VMName] to destroy the migration VM on
  destination immediately.
step 3: the destination libvirtd will be probably deadlock or segmentfault.

Deadlock stack as followed:
#0  0x7fb5c18132d4 in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x7fb5c180e659 in _L_lock_1008 () from /lib64/libpthread.so.0
#2  0x7fb5c180e46e in pthread_mutex_lock () from /lib64/libpthread.so.0
#3  0x7fb5c45d175f in virMutexLock (m=0x7fb5b0066ed0) at util/virthread.c:88
#4  0x7fb5c45b6b04 in virObjectLock (anyobj=0x7fb5b0066ec0) at
util/virobject.c:323
#5  0x7fb5b8f4842a in qemuMonitorEmitEvent (mon=0x7fb5b0066ec0,
event=0x7fb5b00688d0 SHUTDOWN, seconds=1399374472, micros=509994,
details=0x0) at qemu/qemu_monitor.c:1185
#6  0x7fb5b8f62af2 in qemuMonitorJSONIOProcessEvent (mon=0x7fb5b0066ec0,
obj=0x7fb5b0069080) at qemu/qemu_monitor_json.c:158
#7  0x7fb5b8f62d25 in qemuMonitorJSONIOProcessLine (mon=0x7fb5b0066ec0,
line=0x7fb5b005bbe0 {\timestamp\: {\seconds\: 1399374472,
\microseconds\: 509994}, \event\: \SHUTDOWN\},msg=0x7fb5bd873c80)
at qemu/qemu_monitor_json.c:195
#8  0x7fb5b8f62f85 in qemuMonitorJSONIOProcess (mon=0x7fb5b0066ec0,
data=0x7fb5b0060770 {\timestamp\: {\seconds\: 1399374472,
\microseconds\: 509994},\event\: \SHUTDOWN\}\r\n, len=85,
msg=0x7fb5bd873c80) at qemu/qemu_monitor_json.c:237
#9  0x7fb5b8f49aa0 in qemuMonitorIOProcess (mon=0x7fb5b0066ec0)
at qemu/qemu_monitor.c:402
#10 0x7fb5b8f4a09b in qemuMonitorIO (watch=20, fd=24, events=0,
opaque=0x7fb5b0066ec0) at qemu/qemu_monitor.c:651
#11 0x7fb5c458c4d9 in virEventPollDispatchHandles (nfds=17, 
fds=0x7fb5b0068a60)
at util/vireventpoll.c:510
#12 0x7fb5c458decf in virEventPollRunOnce () at util/vireventpoll.c:659
#13 0x7fb5c458bfcc in virEventRunDefaultImpl () at util/virevent.c:308
#14 0x7fb5c51a17a9 in virNetServerRun (srv=0x7fb5c5411d70)
at rpc/virnetserver.c:1139
#15 0x7fb5c5157f63 in main (argc=3, argv=0x7fff7fc04f48) at libvirtd.c:150


After analysis, I found it may be caused by multithreaded simultaneously
access to the global variables vm-privateData-mon.
When problems occur，there are three libvirtd threads at work on destination
host，suppose：
ThreadA: migration thread，do qemuProcessStart.
ThreadB: destroy thread，do qemuDoaminDestroy - qemuProcessStop.
ThreadC：Monitor Thread，do IOWrite、IORead and some other operations according to
the mon-msg when mon-fd change. When threadB destroy happpens, this thread 
would
handle the SUHTDOWN event.

In threadA, when it sends QMP command to Qemu, it will operate the 
vm-privateData-mon
lock. Such as the operation qemuDomainObjEnterMonitor - qemuMonitorSetBalloon 
-
qemuDomainObjExitMonitor, but it's not an atomic operation. If virsh destroy 
[VMName]
happens during this operation, threadB will set the lock vm-privateData-mon 
to NULL in
qemuProcessStop. And then in threadA, the function qemuDomainObjExitMonitor 
will fail to
unlock vm-privateData-mon as it's NULL. So, threadC will never acquire the
vm-privateData-mon lock and the deadlock problem happened.

what was worse, if qemuMonitorSetBalloon perform succeed in threadA.
ThreadA will coutinue to execute till it enter the function 
qemuMonitorSetDomainLog,
it would cause segmentfault at VIR_FORCE_CLOSE(mon-logfd) due to mon is NULL.

I could not find a good way to sovle this problem. Does anyone have good ideas?
Thanks.

Ps:
I find an easy way to reproduce this problem  more probably by following steps:

step 1: Fault Injection, fit into this patch and update the libvirtd on 
destination host:
--- src/qemu/qemu_process.c 2014-05-06 19:06:00.0 +0800
+++ src/qemu/qemu_process.c 2014-05-06 19:07:12.0 +0800
@@ -4131,6 +4131,8 @@
vm-def-mem.cur_balloon);
 goto cleanup;
 }
+VIR_DEBUG(Fault Injection, sleep 3 seconds.);
+sleep(3);
 qemuDomainObjEnterMonitor(driver, vm);
 if (vm-def-memballoon  vm-def-memballoon-period)
 qemuMonitorSetMemoryStatsPeriod(priv-mon, 
vm-def-memballoon-period);

step 2: migrate VM.
step 3: execute virsh destroy [VMName] to destroy the migration VM on 
destination
when log prints Fault Injection, sleep 3 seconds.
step 4: the libvirtd deadlock stack occurs as above mentioned.

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

[libvirt] Configure virtio-scsi options via libvirt

2014-05-06 Thread Mike Perez

Hi everyone,

I would like be able to configure virtio-scsi options num_queues, max_sectors,
and cmd_per_lun via libvirt. Are there any plans to have this support?

-- 
Mike Perez

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

Re: [libvirt] [libvirt-users] Virsh Attach Volume Fails

2014-05-06 Thread Vilobh Meshram

The contents of disk.xml
disk type='network' device=disk
driver name=qemu type=raw cache=writeback/
source protocol=rbd
name=volumes/volume-3cd6fc4d-3855-49ca-820c-2e0db8f45085
host name='68.142.237.36' port='6789'/
/source
target dev=vdg bus=virtio/
auth username='client.admin'
secret type='ceph' uuid='c621f0e9-49fc-4ac2-82a9-34aa80122e33'/
/auth
/disk



Trying to attach a volume created on CEPH RBD backend to an VM created.
Can¹t see meaningful information in QEMU log nor in libvirt logs.

I have the needed CEPH libs installed like librbd, librados.

On 5/6/14, 4:35 PM, Eric Blake ebl...@redhat.com wrote:

On 05/06/2014 02:51 AM, Vilobh Meshram wrote:
 Hi,
 
 When trying to attach a created volume to an instance using the virsh
utility I am running into this error :-
 
 
 [] $ sudo virsh attach-device instance-0015 disk.xml
 
 error: Failed to attach device from disk.xml
 
 error: internal error unable to execute QEMU command
'__com.redhat_drive_add': Device 'drive-virtio-disk21' could not be
initialized

That's an error message from RHEL qemu, and not available from upstream
qemu nor triggered by upstream libvirt (that is, a RHEL-specific
downstream patch to libvirt is attempting to use a downstream qemu
command that upstream does not have).  Have you opened a support ticket
with Red Hat?

 
 
 For experimental purpose I have libvirt 0.10.2 and QEMU 0.12.1. The
volume to be attached is on a remote storage.

This list is probably better suited to help if you can reproduce with
upstream versions (libvirt 1.2.4 and qemu 2.0), or at a bare minimum,
post the contents of disk.xml that is failing on you.  It's hard to say
whether the error is in libvirt (perhaps botched SELinux permissions
getting in the way?) or in qemu; but since libvirt is just parroting
qemu's error message, the investigation needs to look more at why qemu
is throwing the message.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH RFC 02/22] storage: Store gluster volume name separately

On 05/06/2014 07:36 AM, Peter Krempa wrote:
 The gluster volume name was previously stored as part of the source path
 string. This isn't a good idea when we want to start to deal with
 resolving of relative paths on gluster as a storage volume can't
 actually be changed by ../-ing it.
 
 Parse and store the volume name separately for gluster storage volumes
 and use the newly stored variable appropriately.
 ---
  src/conf/domain_conf.c| 33 -
  src/qemu/qemu_command.c   | 19 ++-
  src/storage/storage_backend_gluster.c | 27 ---
  src/util/virstoragefile.c |  1 +
  src/util/virstoragefile.h |  1 +
  5 files changed, 56 insertions(+), 25 deletions(-)
 

ACK

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH v7 3/6] qemu: track quiesced status in qemuDomainSnapshotFSFreeze

On 05/01/2014 06:06 PM, Tomoki Sekiyama wrote:
 Adds 'quiesced' status into qemuDomainObjPrivate that tracks whether
 FSFreeze is requested in the domain.
 
 It modifies error code from qemuDomainSnapshotFSFreeze and
 qemuDomainSnapshotFSThaw, so that a caller can know whether the command is
 actually sent to the guest agent. If the error is caused before sending a
 freeze command, a counterpart thaw command shouldn't be sent either, not to
 confuse fsfreeze status tracking.
 
 Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@hds.com
 ---
  src/qemu/qemu_domain.c |5 +++
  src/qemu/qemu_domain.h |2 +
  src/qemu/qemu_driver.c |   70 
 +++-
  3 files changed, 64 insertions(+), 13 deletions(-)
 

 -qemuDomainSnapshotFSFreeze(virDomainObjPtr vm)
 +qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver,
 +   virDomainObjPtr vm)
  {
  qemuDomainObjPrivatePtr priv = vm-privateData;
 +virQEMUDriverConfigPtr cfg;
  int freezed;

Pre-existing, but we should do a cleanup of s/freezed/frozen/

Pushed per Dan's ack

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH RFC 03/22] storage: Rework debugging of storage file access through storage driver

On 05/06/2014 07:36 AM, Peter Krempa wrote:
 Print the debug statements of individual file access functions from the
 main API functions instead of the individual backend functions.
 
 Also enhance initialization debug messages on a per-backend basis.
 ---
  src/storage/storage_backend_fs.c  | 43 
 ++-
  src/storage/storage_backend_gluster.c | 30 +---
  src/storage/storage_driver.c  | 27 +++---
  3 files changed, 62 insertions(+), 38 deletions(-)
 

ACK

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH RFC 04/22] conf: Fix domain disk path iterator to work with networked storage

On 05/06/2014 07:36 AM, Peter Krempa wrote:
 Skip networked storage but continue iteration through backing chain to
 iterate through all the local paths in the backing chain.
 ---
  src/conf/domain_conf.c | 37 -
  1 file changed, 20 insertions(+), 17 deletions(-)
 

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH RFC 05/22] storage: Add NONE protocol type for network disks