Re: [libvirt] [PATCH] virsh: add --start option to the define command

2013-01-09 Thread Viktor Mihajlovski

On 01/09/2013 12:41 AM, Eric Blake wrote:


But if we do that, it would argue that 'virsh create --persistent
blah.xml' is nicer than 'virsh define --start blah.xml', at least in
that the former needs only 1 API call for new libvirt (but falls back to
2 API calls when talking to older libvirt), while the latter always
needs 2 API calls.

virsh create --persistent is more natural to me especially w.r.t
usability, i.e. virsh create --persistent --console blah.xml vs.
virsh define --start --console blah.xml. (also consider --paused,
--autodestroy).

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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


[libvirt] [PATCH] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

2013-01-09 Thread Gao feng
when we have no host's src mapped to container's root.
there is not .oldroot dir,we should pass / to
lxcContainerMountProcFuse in function
lxcContainerSetupExtraMounts.

Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
 src/lxc/lxc_container.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index d234426..09a4365 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr 
vmDef,
 goto cleanup;
 
 /* Mounts /proc/meminfo etc sysinfo */
-if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
+if (lxcContainerMountProcFuse(vmDef, /)  0)
 goto cleanup;
 
 /* Now we can re-mount the cgroups controllers in the
-- 
1.7.11.7

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


Re: [libvirt] [PATCH] Add RESUME event listener to qemu monitor.

2013-01-09 Thread Michal Privoznik
On 08.01.2013 21:08, Andres Lagar-Cavilla wrote:
 On Jan 8, 2013, at 1:30 PM, Daniel P. Berrange berra...@redhat.com wrote:

snip/


 ACK to your patch anyway.
 
 Cool, thanks.
 
 Not sure about the process around here. More ACKs needed?
 
 Andres

No, I've pushed the patch now.

Michal

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


Re: [libvirt] [PATCH qom-cpu 2/7] target-i386: Don't set any KVM flag by default if KVM is disabled

2013-01-09 Thread Igor Mammedov
On Mon,  7 Jan 2013 16:20:43 -0200
Eduardo Habkost ehabk...@redhat.com wrote:

 This is a cleanup that tries to solve two small issues:
 
  - We don't need a separate kvm_pv_eoi_features variable just to keep a
constant calculated at compile-time, and this style would require
adding a separate variable (that's declared twice because of the
CONFIG_KVM ifdef) for each feature that's going to be enabled/disable
by machine-type compat code.
  - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features
even when KVM is disabled at runtime. This small incosistency in
the cpuid_kvm_features field isn't a problem today because
cpuid_kvm_features is ignored by the TCG code, but it may cause
unexpected problems later when refactoring the CPUID handling code.
 
 This patch eliminates the kvm_pv_eoi_features variable and simply uses
 kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it
 enables kvm_pv_eoi only if KVM is enabled. I believe this makes the
 behavior of enable_kvm_pv_eoi() clearer and easier to understand.

Subj doesn't match what patch actually does.
Have you meant Don't set kvm_pv_eoi flag by default if KVM is disabled?
Although eliminate kvm_pv_eoi_features variable might better describe what
patch is doing.

 
 Signed-off-by: Eduardo Habkost ehabk...@redhat.com
 ---
 Cc: k...@vger.kernel.org
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Gleb Natapov g...@redhat.com
 Cc: Marcelo Tosatti mtosa...@redhat.com
 
 Changes v2:
  - Coding style fix
 
 Changes v3:
  - Eliminate #ifdef by using the fake KVM_FEATURE_PV_EOI #define
 ---
  target-i386/cpu.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 951e206..40400ac 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -164,15 +164,15 @@ static uint32_t kvm_default_features = (1 
 KVM_FEATURE_CLOCKSOURCE) | (1  KVM_FEATURE_ASYNC_PF) |
  (1  KVM_FEATURE_STEAL_TIME) |
  (1  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
 -static const uint32_t kvm_pv_eoi_features = (0x1  KVM_FEATURE_PV_EOI);
  #else
  static uint32_t kvm_default_features = 0;
 -static const uint32_t kvm_pv_eoi_features = 0;
  #endif
  
  void enable_kvm_pv_eoi(void)
  {
 -kvm_default_features |= kvm_pv_eoi_features;
 +if (kvm_enabled()) {
 +kvm_default_features |= (1UL  KVM_FEATURE_PV_EOI);
 +}
  }
  
  void host_cpuid(uint32_t function, uint32_t count,

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


Re: [libvirt] [PATCH] Make TLS support conditional

2013-01-09 Thread Daniel P. Berrange
On Tue, Jan 08, 2013 at 04:30:49PM -0700, Eric Blake wrote:
 On 01/08/2013 01:47 PM, Daniel P. Berrange wrote:
  On Mon, Jan 07, 2013 at 05:37:30PM -0700, Eric Blake wrote:
 
  Touches quite a bit, but hopefully for the better.  What platform are
  you targeting where you were unwilling to require gnutls as a prereq?
  
  No specific platform as such, just that if you build with
  --without-remote and --without-libvirtd we should not be
  mandating use of gnutls. Various people have asked for this
  feature over the years, so I think it is worth it.
  
 
  Overall, your patch looks sane, and you have a 'weak ACK' - that is, I'm
  willing to look the other way and let this patch go in, if you don't
  think it is worth even more refactoring to avoid quite so much leaky
  #ifdef throughout the code base.
  
  Basically I'm following the approach used for SASL. It would be nice to
  try and adapt virnet{tls,sasl}context.c so that all the functions still
  exist, but have no-op impls, but that's much more work - I've tried it
  before with SASL but never got a satisfactory result
 
 As it is, with your patch, I just got this failure on RHEL 5:
 
 /usr/bin/perl ./check-symfile.pl l ibvirt.yms \
 .libs/libvirt.so
 Expected symbol virNetServerClientGetTLSKeySize is not in ELF library
 ...
 
 I still need to do more investigation, but it makes me wonder if we got
 the conditional symfile manipulation correct?

Yeah, actually I think that's something I forgot to handle. That said
on RHEL5, GNUTLS should be present so that symbol ought to have been
built, unless you were testing with --without-gnutls perhaps ?


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] virsh: add --start option to the define command

2013-01-09 Thread Daniel P. Berrange
On Tue, Jan 08, 2013 at 04:41:58PM -0700, Eric Blake wrote:
 On 01/08/2013 02:36 PM, Doug Goldstein wrote:
  I often find myself doing virsh define blah.xml; start blah. I figured
  adding this would be a easier^Hlazier way to do it.
  ---
   tools/virsh-domain.c |   20 ++--
   1 files changed, 18 insertions(+), 2 deletions(-)
 
 Offhand, I like it.  However,
 
 We have virDomainDefineXML with no flags, but we have virDomainCreateXML
 with flags; maybe the better approach is to add a new creation flag that
 says that in addition to starting the domain, we also make it persistent
 at the same time.
 
 But if we do that, it would argue that 'virsh create --persistent
 blah.xml' is nicer than 'virsh define --start blah.xml', at least in
 that the former needs only 1 API call for new libvirt (but falls back to
 2 API calls when talking to older libvirt), while the latter always
 needs 2 API calls.
 
 Or maybe it means we need to add virDomainDefineXMLFlags().
 
 Anyone else want to throw some paint on the bikeshed on how best to make
 the user experience nicer?

While I think the virsh idea is fine, I don't want to see this done
at the virDomainDefine API level, since it just duplicates functionality
already present. Just have virsh make an API call to the existing create
API.

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 1/2] add qemu pci-express device support

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 10:35:32AM +0800, liguang wrote:
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/conf/device_conf.c   |8 +++-
  src/conf/device_conf.h   |1 +
  src/qemu/qemu_capabilities.c |   11 +++
  src/qemu/qemu_capabilities.h |1 +
  src/qemu/qemu_command.c  |4 +++-
  5 files changed, 23 insertions(+), 2 deletions(-)
 
 diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
 index 7b97f45..3fca853 100644
 --- a/src/conf/device_conf.c
 +++ b/src/conf/device_conf.c
 @@ -51,7 +51,7 @@ int
  virDevicePCIAddressParseXML(xmlNodePtr node,
  virDevicePCIAddressPtr addr)
  {
 -char *domain, *slot, *bus, *function, *multi;
 +char *domain, *slot, *bus, *function, *multi, *expr;
  int ret = -1;
  
  memset(addr, 0, sizeof(*addr));
 @@ -61,6 +61,7 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
  slot = virXMLPropString(node, slot);
  function = virXMLPropString(node, function);
  multi= virXMLPropString(node, multifunction);
 +expr = virXMLPropString(node, express);

NACK, this is a gross hack.

The q35 machine type provides multiple PCI buses, and it is
already possible to express connection to the alternative
buses using the 'bus' parameter. We don't need a new 'express'
parameter. We need to make sure that the XML includes a
controller element for each PCI bus provided by a machine
type

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] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 04:05:58PM +0800, Gao feng wrote:
 when we have no host's src mapped to container's root.
 there is not .oldroot dir,we should pass / to
 lxcContainerMountProcFuse in function
 lxcContainerSetupExtraMounts.
 
 Signed-off-by: Gao feng gaof...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index d234426..09a4365 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr 
 vmDef,
  goto cleanup;
  
  /* Mounts /proc/meminfo etc sysinfo */
 -if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
 +if (lxcContainerMountProcFuse(vmDef, /)  0)
  goto cleanup;
  
  /* Now we can re-mount the cgroups controllers in the

No, we should be passing NULL in that case, and make the method
handle NULL correctly.


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 2/2] build acpitable argument for qemu

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 10:35:33AM +0800, liguang wrote:
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/conf/domain_conf.c  |1 +
  src/conf/domain_conf.h  |1 +
  src/qemu/qemu_command.c |4 
  3 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 6a7646e..54ba77f 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -9548,6 +9548,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
 caps,
  
  def-os.bootloader = virXPathString(string(./bootloader), ctxt);
  def-os.bootloaderArgs = virXPathString(string(./bootloader_args), 
 ctxt);
 +def-os.acpitable = virXPathString(string(./bootloader_args), ctxt);
  

This is clearly bogus - you can't just grab an existing XML field and
repurpose it. Second we shouldn't be requireing the user to specify
custom ACPI tables just to use the machine type. libvirt should do the
right thing with q35.

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] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

2013-01-09 Thread Gao feng
On 2013/01/09 18:33, Daniel P. Berrange wrote:
 On Wed, Jan 09, 2013 at 04:05:58PM +0800, Gao feng wrote:
 when we have no host's src mapped to container's root.
 there is not .oldroot dir,we should pass / to
 lxcContainerMountProcFuse in function
 lxcContainerSetupExtraMounts.

 Signed-off-by: Gao feng gaof...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index d234426..09a4365 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -2059,7 +2059,7 @@ static int 
 lxcContainerSetupExtraMounts(virDomainDefPtr vmDef,
  goto cleanup;
  
  /* Mounts /proc/meminfo etc sysinfo */
 -if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
 +if (lxcContainerMountProcFuse(vmDef, /)  0)
  goto cleanup;
  
  /* Now we can re-mount the cgroups controllers in the
 
 No, we should be passing NULL in that case, and make the method
 handle NULL correctly.
 

You mean this?

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index d234426..9f22923 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,

 if ((ret = virAsprintf(meminfo_path,
%s/%s/%s/meminfo,
-   srcprefix, LXC_STATE_DIR,
+   srcprefix ? srcprefix : , LXC_STATE_DIR,
def-name))  0)
 return ret;

@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vm
Def,
 goto cleanup;

 /* Mounts /proc/meminfo etc sysinfo */
-if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
+if (lxcContainerMountProcFuse(vmDef, NULL)  0)
 goto cleanup;

 /* Now we can re-mount the cgroups controllers in the

Thanks!


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


Re: [libvirt] [PATCH] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 06:45:48PM +0800, Gao feng wrote:
 On 2013/01/09 18:33, Daniel P. Berrange wrote:
  On Wed, Jan 09, 2013 at 04:05:58PM +0800, Gao feng wrote:
  when we have no host's src mapped to container's root.
  there is not .oldroot dir,we should pass / to
  lxcContainerMountProcFuse in function
  lxcContainerSetupExtraMounts.
 
  Signed-off-by: Gao feng gaof...@cn.fujitsu.com
  ---
   src/lxc/lxc_container.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
  index d234426..09a4365 100644
  --- a/src/lxc/lxc_container.c
  +++ b/src/lxc/lxc_container.c
  @@ -2059,7 +2059,7 @@ static int 
  lxcContainerSetupExtraMounts(virDomainDefPtr vmDef,
   goto cleanup;
   
   /* Mounts /proc/meminfo etc sysinfo */
  -if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
  +if (lxcContainerMountProcFuse(vmDef, /)  0)
   goto cleanup;
   
   /* Now we can re-mount the cgroups controllers in the
  
  No, we should be passing NULL in that case, and make the method
  handle NULL correctly.
  
 
 You mean this?
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index d234426..9f22923 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
 
  if ((ret = virAsprintf(meminfo_path,
 %s/%s/%s/meminfo,
 -   srcprefix, LXC_STATE_DIR,
 +   srcprefix ? srcprefix : , LXC_STATE_DIR,
 def-name))  0)
  return ret;
 
 @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr 
 vm
 Def,
  goto cleanup;
 
  /* Mounts /proc/meminfo etc sysinfo */
 -if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
 +if (lxcContainerMountProcFuse(vmDef, NULL)  0)
  goto cleanup;
 
  /* Now we can re-mount the cgroups controllers in the
 

ACK

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


[libvirt] [PATCH RESEND] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

2013-01-09 Thread Gao feng
when we has no host's src mapped to container.
there is no .oldroot dir,so libvirt lxc will fail
to start when mouting meminfo.

in this case,the parameter srcprefix of function
lxcContainerMountProcFuse should be NULL.and make
this method handle NULL correctly.

Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
 src/lxc/lxc_container.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index d234426..9f22923 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
 
 if ((ret = virAsprintf(meminfo_path,
%s/%s/%s/meminfo,
-   srcprefix, LXC_STATE_DIR,
+   srcprefix ? srcprefix : , LXC_STATE_DIR,
def-name))  0)
 return ret;
 
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr 
vmDef,
 goto cleanup;
 
 /* Mounts /proc/meminfo etc sysinfo */
-if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
+if (lxcContainerMountProcFuse(vmDef, NULL)  0)
 goto cleanup;
 
 /* Now we can re-mount the cgroups controllers in the
-- 
1.7.11.7

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


Re: [libvirt] [PATCH 0/3]add usb-net support for qemu

2013-01-09 Thread Guannan Ren

On 01/09/2013 04:09 AM, Daniel P. Berrange wrote:

On Tue, Jan 08, 2013 at 03:03:23PM -0500, Laine Stump wrote:

On 01/03/2013 02:13 AM, Guannan Ren wrote:

The set of patches fixed some typo in network docs and codes and
is trying to support usb-net qemu virtual device.

The following is an example for use.

Libvirt XML sample:
   devices
 interface type='user'
   mac address='52:54:00:32:6a:91'/
   model type='usb-net'/

Do we really want the model type to be usb-net? the -net part is
already implicit in the type of device, and there is no requirement that
it needs to exactly match the qemu commandline parameter (although I
suppose that's normally the case). As a matter of fact, type
model='virtio'/ ends up being equivalent to -device virtio-net-pci.

Indeed, we should really base the name on the actual hardware
since there can be many many different USB NICs implemented.

Finding a model name in the usb.ids file is a good place to
start

Daniel


   Okay, I am going to make a v2.
   Thanks for the review

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


Re: [libvirt] [PATCH qom-cpu 2/7] target-i386: Don't set any KVM flag by default if KVM is disabled

2013-01-09 Thread Eduardo Habkost
On Wed, Jan 09, 2013 at 10:46:12AM +0100, Igor Mammedov wrote:
 On Mon,  7 Jan 2013 16:20:43 -0200
 Eduardo Habkost ehabk...@redhat.com wrote:
 
  This is a cleanup that tries to solve two small issues:
  
   - We don't need a separate kvm_pv_eoi_features variable just to keep a
 constant calculated at compile-time, and this style would require
 adding a separate variable (that's declared twice because of the
 CONFIG_KVM ifdef) for each feature that's going to be enabled/disable
 by machine-type compat code.
   - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features
 even when KVM is disabled at runtime. This small incosistency in
 the cpuid_kvm_features field isn't a problem today because
 cpuid_kvm_features is ignored by the TCG code, but it may cause
 unexpected problems later when refactoring the CPUID handling code.
  
  This patch eliminates the kvm_pv_eoi_features variable and simply uses
  kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it
  enables kvm_pv_eoi only if KVM is enabled. I believe this makes the
  behavior of enable_kvm_pv_eoi() clearer and easier to understand.
 
 Subj doesn't match what patch actually does.
 Have you meant Don't set kvm_pv_eoi flag by default if KVM is disabled?
 Although eliminate kvm_pv_eoi_features variable might better describe what
 patch is doing.


True. The other flags in kvm_default_features may be already set and
will be copied to cpuid_kvm_features even if kvm_enabled() is false.

I had a previous version of this patch that also changed the code using
kvm_default_features to check kvm_enabled(), but I removed that part and
kept only the kvm_pv_eoi_features variable removal.

Andreas, please ignore this patch (it is not necessary anymore as this
series doesn't include machine-type compatibility code for kvm_mmu).
Patches 2-7 don't depend on this patch and should apply cleanly without
it.

I will send a new version later, probably with a separate patch to
ignore kvm_default_features if kvm_enabled() is false (so there's no
need to even check kvm_enabled() inside enable_kvm_pv_eoi()).

 
  
  Signed-off-by: Eduardo Habkost ehabk...@redhat.com
  ---
  Cc: k...@vger.kernel.org
  Cc: Michael S. Tsirkin m...@redhat.com
  Cc: Gleb Natapov g...@redhat.com
  Cc: Marcelo Tosatti mtosa...@redhat.com
  
  Changes v2:
   - Coding style fix
  
  Changes v3:
   - Eliminate #ifdef by using the fake KVM_FEATURE_PV_EOI #define
  ---
   target-i386/cpu.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
  
  diff --git a/target-i386/cpu.c b/target-i386/cpu.c
  index 951e206..40400ac 100644
  --- a/target-i386/cpu.c
  +++ b/target-i386/cpu.c
  @@ -164,15 +164,15 @@ static uint32_t kvm_default_features = (1 
  KVM_FEATURE_CLOCKSOURCE) | (1  KVM_FEATURE_ASYNC_PF) |
   (1  KVM_FEATURE_STEAL_TIME) |
   (1  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
  -static const uint32_t kvm_pv_eoi_features = (0x1  KVM_FEATURE_PV_EOI);
   #else
   static uint32_t kvm_default_features = 0;
  -static const uint32_t kvm_pv_eoi_features = 0;
   #endif
   
   void enable_kvm_pv_eoi(void)
   {
  -kvm_default_features |= kvm_pv_eoi_features;
  +if (kvm_enabled()) {
  +kvm_default_features |= (1UL  KVM_FEATURE_PV_EOI);
  +}
   }
   
   void host_cpuid(uint32_t function, uint32_t count,
 

-- 
Eduardo

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


Re: [libvirt] [PATCH qom-cpu 2/7] target-i386: Don't set any KVM flag by default if KVM is disabled

2013-01-09 Thread Eduardo Habkost
On Wed, Jan 09, 2013 at 09:41:37AM -0200, Eduardo Habkost wrote:
[...]
 Andreas, please ignore this patch (it is not necessary anymore as this
 series doesn't include machine-type compatibility code for kvm_mmu).
 Patches 2-7 don't depend on this patch and should apply cleanly without
 it.

I mean: patches 1 and 3-7 don't depend on this patch and can be applied
cleanly without it.

-- 
Eduardo

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


[libvirt] [PATCH] libvirt: lxc: don't mkdir when selinux is disabled

2013-01-09 Thread Gao feng
libvirt lxc will fail to start when selinux is disabled.
error: Failed to start domain noroot
error: internal error guest failed to start: PATH=/bin:/sbin TERM=linux 
container=lxc-libvirt container_uuid=b9873916-3516-c199-8112-1592ff694a9e 
LIBVIRT_LXC_UUID=b9873916-3516-c199-8112-1592ff694a9e LIBVIRT_LXC_NAME=noroot 
/bin/sh
2013-01-09 11:04:05.384+: 1: info : libvirt version: 1.0.1
2013-01-09 11:04:05.384+: 1: error : lxcContainerMountBasicFS:546 : Failed 
to mkdir /sys/fs/selinux: No such file or directory
2013-01-09 11:04:05.384+: 7536: info : libvirt version: 1.0.1
2013-01-09 11:04:05.384+: 7536: error : virLXCControllerRun:1466 : error 
receiving signal from container: Input/output error
2013-01-09 11:04:05.404+: 7536: error : virCommandWait:2287 : internal 
error Child process (ip link del veth1) unexpected exit status 1: Cannot find 
device veth1

fix this problem by checking if selinuxfs is mounted
in host before we try to create dir /sys/fs/selinux.

Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
 src/lxc/lxc_container.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 9f22923..d7f4960 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -540,13 +540,6 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
 VIR_DEBUG(Processing %s - %s,
   mnts[i].src, mnts[i].dst);
 
-if (virFileMakePath(mnts[i].dst)  0) {
-virReportSystemError(errno,
- _(Failed to mkdir %s),
- mnts[i].src);
-goto cleanup;
-}
-
 srcpath = mnts[i].src;
 
 /* Skip if mount doesn't exist in source */
@@ -554,6 +547,13 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
 (access(srcpath, R_OK)  0))
 continue;
 
+if (virFileMakePath(mnts[i].dst)  0) {
+virReportSystemError(errno,
+ _(Failed to mkdir %s),
+ mnts[i].src);
+goto cleanup;
+}
+
 VIR_DEBUG(Mount %s on %s type=%s flags=%x, opts=%s,
   srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags, 
mnts[i].opts);
 if (mount(srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags, 
mnts[i].opts)  0) {
-- 
1.7.11.7

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


[libvirt] [PATCH] build: move file deleting action from %files list to %install

2013-01-09 Thread Yufang Zhang
When building libvirt rpms on rhel5, I got the following error:

File must begin with /: rm
File must begin with /: -f
File must begin with /: $RPM_BUILD_ROOT/etc/sysctl.d/libvirtd
Installed (but unpackaged) file(s) found:
   /etc/sysctl.d/libvirtd

It is triggerd by the %files list of libvirt daemon:

%if 0%{?fedora} = 14 || 0%{?rhel} = 6
%config(noreplace) %{_prefix}/lib/sysctl.d/libvirtd.conf
%else
rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/libvirtd.conf
%endif

After checking document of rpm spec file, I think it would be better
to move the file deleting line from %files list to %install script.
---
 libvirt.spec.in |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 52da4f4..4872c07 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1420,6 +1420,10 @@ mv 
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
 %endif
 %endif
 
+%if 0%{?fedora}  14 || 0%{?rhel} == 5
+rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/libvirtd.conf
+%endif
+
 %clean
 rm -fr %{buildroot}
 
@@ -1679,8 +1683,6 @@ fi
 %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf
 %if 0%{?fedora} = 14 || 0%{?rhel} = 6
 %config(noreplace) %{_prefix}/lib/sysctl.d/libvirtd.conf
-%else
-rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/libvirtd.conf
 %endif
 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/
 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/lxc/
-- 
1.7.4.1

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


[libvirt] [PATCH] qemu_agent: Remove agent reference only when disposing it

2013-01-09 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=892079

With current code, if user calls virDomainPMSuspendForDuration()
followed by virDomainDestroy(), the former API checks for qemu agent
presence, which will evaluate as true (if agent is configured). While
talking to qemu agent, the qemu driver is unlocked, so the latter API
starts executing.  However, if machine dies meanwhile, libvirtd gets
EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The
handler clears reference to qemu agent while the destroy API already
holding a reference to it. This leads to NULL dereferencing later in
the code. Therefore, the agent pointer should be set to NULL only if
we are the exclusive owner of it.
---

There's a reproducer in the BZ. It doesn't have to be a windows guest,
I was able to reproduce with F17 guest as well.

 src/qemu/qemu_process.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 938c17e..320c0c6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -133,7 +133,8 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
 virDomainObjLock(vm);
 
 priv = vm-privateData;
-if (priv-agent == agent)
+if (priv-agent == agent 
+!virObjectUnref(priv-agent))
 priv-agent = NULL;
 
 virDomainObjUnlock(vm);
-- 
1.8.0.2

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


Re: [libvirt] [PATCH RESEND] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

2013-01-09 Thread Michal Privoznik
On 09.01.2013 12:01, Gao feng wrote:
 when we has no host's src mapped to container.
 there is no .oldroot dir,so libvirt lxc will fail
 to start when mouting meminfo.
 
 in this case,the parameter srcprefix of function
 lxcContainerMountProcFuse should be NULL.and make
 this method handle NULL correctly.
 
 Signed-off-by: Gao feng gaof...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index d234426..9f22923 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
  
  if ((ret = virAsprintf(meminfo_path,
 %s/%s/%s/meminfo,
 -   srcprefix, LXC_STATE_DIR,
 +   srcprefix ? srcprefix : , LXC_STATE_DIR,
 def-name))  0)
  return ret;
  
 @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr 
 vmDef,
  goto cleanup;
  
  /* Mounts /proc/meminfo etc sysinfo */
 -if (lxcContainerMountProcFuse(vmDef, /.oldroot)  0)
 +if (lxcContainerMountProcFuse(vmDef, NULL)  0)
  goto cleanup;
  
  /* Now we can re-mount the cgroups controllers in the
 

Now pushed. Thanks.

Michal

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


[libvirt] [PATCH] Move comment after enum members

2013-01-09 Thread Claudio Bley
The api builder always associates comments to the last member it read,
not to the current member even if there was a comment for the previous
member and a comma was already seen.

This has the effect that the comment for the previous member gets
overwritten and the current member has no comment at all.

Signed-off-by: Claudio Bley cb...@av-test.de
---
 include/libvirt/libvirt.h.in |   18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 09c89c5..9110fcf 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -127,12 +127,12 @@ typedef enum {
 power management */
 
 #ifdef VIR_ENUM_SENTINELS
-/*
- * NB: this enum value will increase over time as new events are
- * added to the libvirt API. It reflects the last state supported
- * by this version of the libvirt API.
- */
  VIR_DOMAIN_LAST
+ /*
+  * NB: this enum value will increase over time as new events are
+  * added to the libvirt API. It reflects the last state supported
+  * by this version of the libvirt API.
+  */
 #endif
 } virDomainState;
 
@@ -2742,12 +2742,12 @@ typedef enum {
 VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */
 
 #ifdef VIR_ENUM_SENTINELS
+VIR_STORAGE_VOL_WIPE_ALG_LAST
 /*
  * NB: this enum value will increase over time as new algorithms are
  * added to the libvirt API. It reflects the last algorithm supported
  * by this version of the libvirt API.
  */
-VIR_STORAGE_VOL_WIPE_ALG_LAST
 #endif
 } virStorageVolWipeAlgorithm;
 
@@ -2974,12 +2974,12 @@ typedef enum {
 VIR_KEYCODE_SET_RFB= 9,
 
 #ifdef VIR_ENUM_SENTINELS
+VIR_KEYCODE_SET_LAST
 /*
  * NB: this enum value will increase over time as new events are
  * added to the libvirt API. It reflects the last keycode set supported
  * by this version of the libvirt API.
  */
-VIR_KEYCODE_SET_LAST
 #endif
 } virKeycodeSet;
 
@@ -3533,12 +3533,12 @@ typedef enum {
 VIR_SECRET_USAGE_TYPE_CEPH = 2,
 
 #ifdef VIR_ENUM_SENTINELS
+VIR_SECRET_USAGE_TYPE_LAST
 /*
  * NB: this enum value will increase over time as new events are
  * added to the libvirt API. It reflects the last secret owner ID
  * supported by this version of the libvirt API.
  */
-VIR_SECRET_USAGE_TYPE_LAST
 #endif
 } virSecretUsageType;
 
@@ -4443,12 +4443,12 @@ typedef enum {
 VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* 
virConnectDomainEventPMSuspendDiskCallback */
 
 #ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_EVENT_ID_LAST
 /*
  * NB: this enum value will increase over time as new events are
  * added to the libvirt API. It reflects the last event ID supported
  * by this version of the libvirt API.
  */
-VIR_DOMAIN_EVENT_ID_LAST
 #endif
 } virDomainEventID;
 
-- 
1.7.9.5

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


[libvirt] [PATCH 0/2] Colorize HTML documentation

2013-01-09 Thread Claudio Bley
Hi.

This patchset adds a few classes to the generated HTML documentation.

The style sheets are also adapted making use of the new classes to
give the documentation a little visual overhaul.

YMMV, but it looks good for me using Firefox, Webkit and Opera.

I did check the CSS rules using the W3C CSS validator.

Claudio Bley (2):
  docs: Assign classes to documentation elements
  docs: Add some style and color to the HTML documentation

 docs/generic.css |4 ++
 docs/libvirt.css |   56 +++-
 docs/newapi.xsl  |  187 +++---
 3 files changed, 166 insertions(+), 81 deletions(-)

-- 
1.7.9.5

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


[libvirt] [PATCH 1/2] docs: Assign classes to documentation elements

2013-01-09 Thread Claudio Bley
In CSS the following class names are available:

* keyword (keywords like typedef, struct)
* type(types like int, void*)
* comment (comments after members of enums or structs)
* directive   (preprocessor directives, #define)
* undisclosed (text saying that the API is not public)

Additionally, kill all of the left-over programlisting class
assignments. There are no CSS rules for them.

Signed-off-by: Claudio Bley cb...@av-test.de
---
 docs/newapi.xsl |  187 ---
 1 file changed, 108 insertions(+), 79 deletions(-)

diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index 24831ee..3982f8f 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -116,16 +116,18 @@
   /xsl:template
 
   xsl:template match=macro mode=toc
-xsl:text#define /xsl:text
+span class=directive#define/spanxsl:text /xsl:text
 a href=#{@name}xsl:value-of select=@name//a
 xsl:text
 /xsl:text
   /xsl:template
 
   xsl:template match=variable mode=toc
-xsl:call-template name=dumptext
-  xsl:with-param name=text select=string(@type)/
-/xsl:call-template
+span class=type
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=string(@type)/
+  /xsl:call-template
+/span
 xsl:text /xsl:text
 a name={@name}/a
 xsl:value-of select=@name/
@@ -134,18 +136,21 @@
   /xsl:template
 
   xsl:template match=typedef mode=toc
-xsl:texttypedef /xsl:textxsl:variable name=name 
select=string(@name)/
+span class=keywordtypedef/span
+xsl:text /xsl:textxsl:variable name=name select=string(@name)/
 xsl:choose
   xsl:when test=@type = 'enum'
-xsl:textenum /xsl:text
+span class=keywordenum/spanxsl:text /xsl:text
a href=#{$name}xsl:value-of select=$name//a
xsl:text
 /xsl:text
   /xsl:when
   xsl:otherwise
-   xsl:call-template name=dumptext
- xsl:with-param name=text select=@type/
-   /xsl:call-template
+   span class=type
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=@type/
+  /xsl:call-template
+/span
xsl:text /xsl:text
a name={$name}xsl:value-of select=$name//a
xsl:text
@@ -159,7 +164,7 @@
 h3a name={$name}codexsl:value-of select=$name//code/a/h3
 div class=api
   pre
-xsl:textenum /xsl:text
+span class=keywordenum/spanxsl:text /xsl:text
 xsl:value-of select=$name/
 xsl:text {
 /xsl:text
@@ -173,10 +178,11 @@
 tdxsl:value-of select=@value//td
 xsl:if test=@info != ''
   td
-xsl:text : /xsl:text
-xsl:call-template name=dumptext
-  xsl:with-param name=text select=@info/
-/xsl:call-template
+div class=comment
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=@info/
+  /xsl:call-template
+/div
   /td
 /xsl:if
   /tr
@@ -190,8 +196,8 @@
   /xsl:template
 
   xsl:template match=struct mode=toc
-xsl:texttypedef /xsl:text
-xsl:value-of select=@type/
+span class=keywordtypedef/spanxsl:text /xsl:text
+span class=typexsl:value-of select=@type//span
 xsl:text /xsl:text
 a href=#{@name}xsl:value-of select=@name//a
 xsl:text
@@ -202,32 +208,35 @@
 h3a name={@name}codexsl:value-of select=@name//code/a/h3
 div class=api
   pre
-xsl:textstruct /xsl:text
+span class=keywordstruct /span
 xsl:value-of select=@name/
-xsl:text{
+xsl:text {
 /xsl:text
   /pre
   table
 xsl:for-each select=field
   xsl:choose
 xsl:when test='@type = union'
-  trtdunion {/td/tr
+  trtdspan class=keywordunion/span {/td/tr
   tr
   tdtable
   xsl:for-each select=union/field
 tr
   td
-xsl:call-template name=dumptext
-  xsl:with-param name=text select=@type/
-/xsl:call-template
+span class=type
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=@type/
+  /xsl:call-template
+/span
   /td
   tdxsl:value-of select=@name//td
   xsl:if test=@info != ''
 td
-  xsl:text : /xsl:text
-  xsl:call-template name=dumptext
-xsl:with-param name=text select=@info/
-  /xsl:call-template
+  div class=comment
+xsl:call-template name=dumptext
+  xsl:with-param name=text select=@info/
+/xsl:call-template
+  /div
   

[libvirt] [PATCH 2/2] docs: Add some style and color to the HTML documentation

2013-01-09 Thread Claudio Bley

Signed-off-by: Claudio Bley cb...@av-test.de
---
 docs/generic.css |4 
 docs/libvirt.css |   56 --
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/docs/generic.css b/docs/generic.css
index dbf7b56..1def6bf 100644
--- a/docs/generic.css
+++ b/docs/generic.css
@@ -73,3 +73,7 @@ dl dd {
   margin-right: 2em;
   margin-bottom: 0.5em;
 }
+
+tt, pre {
+  font-family: Ubuntu Monospace, Consolas, Lucida Console, monospace;
+}
diff --git a/docs/libvirt.css b/docs/libvirt.css
index 5123ed6..305dedf 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -184,23 +184,25 @@ div.api {
 border: 1px solid #99;
 background: #ee;
 color: black;
+padding: 3px;
 }
 
 div.api pre {
 margin: 0px;
 border: 0px;
 background: inherit;
+padding: inherit;
 }
 
 div.api table {
 margin: 0px;
 padding-left: 2em;
-font-family: fixed;
-whitespace: pre;
+border-spacing: 0px;
 }
 
 div.api table td, div.variablelist table td {
 vertical-align: top;
+padding-left: 1em;
 }
 
 
@@ -412,3 +414,53 @@ table.data tbody td.n {
 background: rgb(255,220,220);
 text-align: center;
 }
+
+.api {
+font-family: Ubuntu Monospace, Consolas, Lucida Console, monospace;
+line-height: 175%;
+}
+
+.api .type {
+font-weight: bold;
+white-space: nowrap;
+color: darkslateblue;
+}
+
+.api .keyword {
+font-weight: bold;
+color: #A2F;
+}
+
+.api .comment {
+color: #080;
+margin-left: 2em;
+position: relative;
+}
+
+.api .comment:before {
+content: : ;
+position: absolute;
+left: -1.3em;
+}
+
+.api .undisclosed {
+font-style: italic;
+letter-spacing: .3ex;
+font-weight: bolder;
+text-transform: uppercase;
+}
+
+.api .directive {
+color: teal;
+}
+
+.api:link:hover, .api:link:focus {
+color: blue;
+border-color: blue;
+}
+
+.api:link {
+text-decoration: none;
+padding-bottom: 2px;
+border-bottom: 1px dashed grey;
+}
-- 
1.7.9.5

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


[libvirt] [PATCH 0/2 v2] Colorize HTML documentation

2013-01-09 Thread Claudio Bley
The only difference to v1 is that I fixed the CSS pseudo elements
adding some vital whitespaces.

(I tried to be extra smart and removed some whitespace without further
 testing, just to discover that it did not work anymore *after*
 sending the patcheset).

Claudio Bley (2):
  docs: Assign classes to documentation elements
  docs: Add some style and color to the HTML documentation

 docs/generic.css |4 ++
 docs/libvirt.css |   56 +++-
 docs/newapi.xsl  |  187 +++---
 3 files changed, 166 insertions(+), 81 deletions(-)

-- 
1.7.9.5

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


[libvirt] [PATCH 2/2 v2] docs: Add some style and color to the HTML documentation

2013-01-09 Thread Claudio Bley

Signed-off-by: Claudio Bley cb...@av-test.de
---
 docs/generic.css |4 
 docs/libvirt.css |   56 --
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/docs/generic.css b/docs/generic.css
index dbf7b56..1def6bf 100644
--- a/docs/generic.css
+++ b/docs/generic.css
@@ -73,3 +73,7 @@ dl dd {
   margin-right: 2em;
   margin-bottom: 0.5em;
 }
+
+tt, pre {
+  font-family: Ubuntu Monospace, Consolas, Lucida Console, monospace;
+}
diff --git a/docs/libvirt.css b/docs/libvirt.css
index 5123ed6..2bd9f8f 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -184,23 +184,25 @@ div.api {
 border: 1px solid #99;
 background: #ee;
 color: black;
+padding: 3px;
 }
 
 div.api pre {
 margin: 0px;
 border: 0px;
 background: inherit;
+padding: inherit;
 }
 
 div.api table {
 margin: 0px;
 padding-left: 2em;
-font-family: fixed;
-whitespace: pre;
+border-spacing: 0px;
 }
 
 div.api table td, div.variablelist table td {
 vertical-align: top;
+padding-left: 1em;
 }
 
 
@@ -412,3 +414,53 @@ table.data tbody td.n {
 background: rgb(255,220,220);
 text-align: center;
 }
+
+.api {
+font-family: Ubuntu Monospace, Consolas, Lucida Console, monospace;
+line-height: 175%;
+}
+
+.api .type {
+font-weight: bold;
+white-space: nowrap;
+color: darkslateblue;
+}
+
+.api .keyword {
+font-weight: bold;
+color: #A2F;
+}
+
+.api .comment {
+color: #080;
+margin-left: 2em;
+position: relative;
+}
+
+.api .comment:before {
+content: : ;
+position: absolute;
+left: -1.3em;
+}
+
+.api .undisclosed {
+font-style: italic;
+letter-spacing: .3ex;
+font-weight: bolder;
+text-transform: uppercase;
+}
+
+.api .directive {
+color: teal;
+}
+
+.api :link:hover, .api :link:focus {
+color: blue;
+border-color: blue;
+}
+
+.api :link {
+text-decoration: none;
+padding-bottom: 2px;
+border-bottom: 1px dashed grey;
+}
-- 
1.7.9.5

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


[libvirt] [PATCH 1/2 v2] docs: Assign classes to documentation elements

2013-01-09 Thread Claudio Bley
In CSS the following class names are available:

* keyword (keywords like typedef, struct)
* type(types like int, void*)
* comment (comments after members of enums or structs)
* directive   (preprocessor directives, #define)
* undisclosed (text saying that the API is not public)

Additionally, kill all of the left-over programlisting class
assignments. There are no CSS rules for them.

Signed-off-by: Claudio Bley cb...@av-test.de
---
 docs/newapi.xsl |  187 ---
 1 file changed, 108 insertions(+), 79 deletions(-)

diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index 24831ee..3982f8f 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -116,16 +116,18 @@
   /xsl:template
 
   xsl:template match=macro mode=toc
-xsl:text#define /xsl:text
+span class=directive#define/spanxsl:text /xsl:text
 a href=#{@name}xsl:value-of select=@name//a
 xsl:text
 /xsl:text
   /xsl:template
 
   xsl:template match=variable mode=toc
-xsl:call-template name=dumptext
-  xsl:with-param name=text select=string(@type)/
-/xsl:call-template
+span class=type
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=string(@type)/
+  /xsl:call-template
+/span
 xsl:text /xsl:text
 a name={@name}/a
 xsl:value-of select=@name/
@@ -134,18 +136,21 @@
   /xsl:template
 
   xsl:template match=typedef mode=toc
-xsl:texttypedef /xsl:textxsl:variable name=name 
select=string(@name)/
+span class=keywordtypedef/span
+xsl:text /xsl:textxsl:variable name=name select=string(@name)/
 xsl:choose
   xsl:when test=@type = 'enum'
-xsl:textenum /xsl:text
+span class=keywordenum/spanxsl:text /xsl:text
a href=#{$name}xsl:value-of select=$name//a
xsl:text
 /xsl:text
   /xsl:when
   xsl:otherwise
-   xsl:call-template name=dumptext
- xsl:with-param name=text select=@type/
-   /xsl:call-template
+   span class=type
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=@type/
+  /xsl:call-template
+/span
xsl:text /xsl:text
a name={$name}xsl:value-of select=$name//a
xsl:text
@@ -159,7 +164,7 @@
 h3a name={$name}codexsl:value-of select=$name//code/a/h3
 div class=api
   pre
-xsl:textenum /xsl:text
+span class=keywordenum/spanxsl:text /xsl:text
 xsl:value-of select=$name/
 xsl:text {
 /xsl:text
@@ -173,10 +178,11 @@
 tdxsl:value-of select=@value//td
 xsl:if test=@info != ''
   td
-xsl:text : /xsl:text
-xsl:call-template name=dumptext
-  xsl:with-param name=text select=@info/
-/xsl:call-template
+div class=comment
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=@info/
+  /xsl:call-template
+/div
   /td
 /xsl:if
   /tr
@@ -190,8 +196,8 @@
   /xsl:template
 
   xsl:template match=struct mode=toc
-xsl:texttypedef /xsl:text
-xsl:value-of select=@type/
+span class=keywordtypedef/spanxsl:text /xsl:text
+span class=typexsl:value-of select=@type//span
 xsl:text /xsl:text
 a href=#{@name}xsl:value-of select=@name//a
 xsl:text
@@ -202,32 +208,35 @@
 h3a name={@name}codexsl:value-of select=@name//code/a/h3
 div class=api
   pre
-xsl:textstruct /xsl:text
+span class=keywordstruct /span
 xsl:value-of select=@name/
-xsl:text{
+xsl:text {
 /xsl:text
   /pre
   table
 xsl:for-each select=field
   xsl:choose
 xsl:when test='@type = union'
-  trtdunion {/td/tr
+  trtdspan class=keywordunion/span {/td/tr
   tr
   tdtable
   xsl:for-each select=union/field
 tr
   td
-xsl:call-template name=dumptext
-  xsl:with-param name=text select=@type/
-/xsl:call-template
+span class=type
+  xsl:call-template name=dumptext
+xsl:with-param name=text select=@type/
+  /xsl:call-template
+/span
   /td
   tdxsl:value-of select=@name//td
   xsl:if test=@info != ''
 td
-  xsl:text : /xsl:text
-  xsl:call-template name=dumptext
-xsl:with-param name=text select=@info/
-  /xsl:call-template
+  div class=comment
+xsl:call-template name=dumptext
+  xsl:with-param name=text select=@info/
+/xsl:call-template
+  /div
   

[libvirt] [PATCH 06/14] rpc: Avoid resource leak of 'socks' if any object append fails

2013-01-09 Thread John Ferlan
---
 src/rpc/virnetserverservice.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 61dd682..5deacaa 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -356,9 +356,6 @@ virJSONValuePtr 
virNetServerServicePreExecRestart(virNetServerServicePtr svc)
 if (!object)
 return NULL;
 
-if (!(socks = virJSONValueNewArray()))
-goto error;
-
 if (virJSONValueObjectAppendNumberInt(object, auth, svc-auth)  0)
 goto error;
 if (virJSONValueObjectAppendBoolean(object, readonly, svc-readonly)  0)
@@ -366,6 +363,9 @@ virJSONValuePtr 
virNetServerServicePreExecRestart(virNetServerServicePtr svc)
 if (virJSONValueObjectAppendNumberUint(object, nrequests_client_max, 
svc-nrequests_client_max)  0)
 goto error;
 
+if (!(socks = virJSONValueNewArray()))
+goto error;
+
 if (virJSONValueObjectAppend(object, socks, socks)  0) {
 virJSONValueFree(socks);
 goto error;
-- 
1.7.11.7

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


[libvirt] [PATCH 05/14] locking: Resolve resource leak with 'dir' on non error path

2013-01-09 Thread John Ferlan
---
 src/locking/lock_driver_sanlock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/locking/lock_driver_sanlock.c 
b/src/locking/lock_driver_sanlock.c
index d06fa66..df6bee9 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -235,6 +235,7 @@ static int virLockManagerSanlockSetupLockspace(void)
path);
 goto error;
 }
+VIR_FREE(dir);
 
 if (driver-group != (gid_t) -1)
 perms |= 0060;
-- 
1.7.11.7

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


[libvirt] [PATCH 07/14] uml: Avoid resource leak of event in umlInofityEvent

2013-01-09 Thread John Ferlan
If there was more than one inotify_event found in the read/while loop,
then only the last event found would have been queued.
---
 src/uml/uml_driver.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 448d292..db6e466 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -410,11 +410,13 @@ reread:
 }
 if (dom)
 virDomainObjUnlock(dom);
+if (event) {
+umlDomainEventQueue(driver, event);
+event = NULL;
+}
 }
 
 cleanup:
-if (event)
-umlDomainEventQueue(driver, event);
 umlDriverUnlock(driver);
 }
 
-- 
1.7.11.7

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


[libvirt] [PATCH 08/14] test: Resource resource leak with 'tmp_vols'

2013-01-09 Thread John Ferlan
---
 src/test/test_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 3e082a4..0e1d90c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4687,6 +4687,7 @@ testStoragePoolListAllVolumes(virStoragePoolPtr obj,
 if (tmp_vols[i])
 virStorageVolFree(tmp_vols[i]);
 }
+VIR_FREE(tmp_vols);
 }
 
 if (pool)
-- 
1.7.11.7

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


[libvirt] [PATCH 10/14] interface: Resolve resource leak wth 'tmp_iface_objs'

2013-01-09 Thread John Ferlan
---
 src/interface/interface_backend_netcf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/interface/interface_backend_netcf.c 
b/src/interface/interface_backend_netcf.c
index 35335c2..8671717 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -399,6 +399,7 @@ cleanup:
 if (tmp_iface_objs[i])
 virInterfaceFree(tmp_iface_objs[i]);
 }
+VIR_FREE(tmp_iface_objs);
 }
 
 interfaceDriverUnlock(driver);
-- 
1.7.11.7

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


[libvirt] [PATCH 11/14] xen: Resource resource leak with 'cpuset'

2013-01-09 Thread John Ferlan
Make cpuset local to the while loop and free it once done with it each
time through the loop.
---
 src/xen/xend_internal.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 84a25e8..c6b800b 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1113,7 +1113,6 @@ sexpr_to_xend_topology(const struct sexpr *root,
 {
 const char *nodeToCpu;
 const char *cur;
-virBitmapPtr cpuset = NULL;
 int *cpuNums = NULL;
 int cell, cpu, nb_cpus;
 int n = 0;
@@ -1131,6 +1130,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
 
 cur = nodeToCpu;
 while (*cur != 0) {
+virBitmapPtr cpuset = NULL;
 /*
  * Find the next NUMA cell described in the xend output
  */
@@ -1152,8 +1152,10 @@ sexpr_to_xend_topology(const struct sexpr *root,
 goto memory_error;
 } else {
 nb_cpus = virBitmapParse(cur, 'n', cpuset, numCpus);
-if (nb_cpus  0)
+if (nb_cpus  0) {
+virBitmapFree(cpuset);
 goto error;
+}
 }
 
 for (n = 0, cpu = 0; cpu  numCpus; cpu++) {
@@ -1163,28 +1165,26 @@ sexpr_to_xend_topology(const struct sexpr *root,
 if (used)
 cpuNums[n++] = cpu;
 }
+virBitmapFree(cpuset);
 
 if (virCapabilitiesAddHostNUMACell(caps,
cell,
nb_cpus,
cpuNums)  0)
 goto memory_error;
+
 }
 VIR_FREE(cpuNums);
-virBitmapFree(cpuset);
 return 0;
 
   parse_error:
 virReportError(VIR_ERR_XEN_CALL, %s, _(topology syntax error));
   error:
 VIR_FREE(cpuNums);
-virBitmapFree(cpuset);
-
 return -1;
 
   memory_error:
 VIR_FREE(cpuNums);
-virBitmapFree(cpuset);
 virReportOOMError();
 return -1;
 }
-- 
1.7.11.7

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


[libvirt] [PATCH 09/14] storage: Resource resource leak using 'tmp_vols'

2013-01-09 Thread John Ferlan
---
 src/storage/storage_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ff56f4f..e98c18c 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1197,6 +1197,7 @@ storagePoolListAllVolumes(virStoragePoolPtr pool,
 if (tmp_vols[i])
 virStorageVolFree(tmp_vols[i]);
 }
+VIR_FREE(tmp_vols);
 }
 
 if (obj)
-- 
1.7.11.7

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


[libvirt] [PATCH 14/14] hyperv: Address resource leak found by Coverityo

2013-01-09 Thread John Ferlan
Because result was used to determine whether or not to free 'priv'
resources Coverity tagged the code as having a resource leak. This
change addresses that concern.
---
 src/hyperv/hyperv_driver.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 601a85a..e69a232 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -121,6 +121,7 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, 
unsigned int flags)
 virReportOOMError();
 goto cleanup;
 }
+conn-privateData = priv;
 
 if (hypervParseUri(priv-parsedUri, conn-uri)  0) {
 goto cleanup;
@@ -199,18 +200,17 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, 
unsigned int flags)
 goto cleanup;
 }
 
-conn-privateData = priv;
-
 result = VIR_DRV_OPEN_SUCCESS;
 
   cleanup:
-if (result == VIR_DRV_OPEN_ERROR) {
-hypervFreePrivate(priv);
-}
+if (result == VIR_DRV_OPEN_ERROR)
+conn-privateData = NULL;
 
 VIR_FREE(username);
 VIR_FREE(password);
 hypervFreeObject(priv, (hypervObject *)computerSystem);
+if (priv  !conn-privateData)
+hypervFreePrivate(priv);
 
 return result;
 }
-- 
1.7.11.7

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


[libvirt] [PATCH 13/14] storage: Need to also VIR_FREE(reg)

2013-01-09 Thread John Ferlan
Commit-id 'afc4631b' added the regfree(reg) to free resources alloc'd
during regcomp; however, reg still needed to be VIR_FREE()'d.
---
 src/storage/storage_backend_logical.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index bd902fe..38b9f08 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -265,6 +265,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
 cleanup:
 VIR_FREE(regex);
 regfree(reg);
+VIR_FREE(reg);
 VIR_FREE(vars);
 if (is_new_vol  (ret == -1))
 virStorageVolDefFree(vol);
-- 
1.7.11.7

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


[libvirt] [PATCH 12/14] parallels: Resolve some resource leaks

2013-01-09 Thread John Ferlan
Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory
label to be consistent

Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is
allocated locally.
---
 src/parallels/parallels_driver.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 6f33080..bf27e54 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -304,8 +304,9 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr 
value)
 
 no_memory:
 virReportOOMError();
-cleanup:
+VIR_FREE(accel);
 virDomainVideoDefFree(video);
+cleanup:
 return -1;
 }
 
@@ -1812,58 +1813,58 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr 
pdom,
 if (!create  oldnet-type != newnet-type) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Changing network type is not supported));
-return -1;
+goto error;
 }
 
 if (!STREQ_NULLABLE(oldnet-model, newnet-model)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Changing network device model is not supported));
-return -1;
+goto error;
 }
 
 if (!STREQ_NULLABLE(oldnet-data.network.portgroup,
 newnet-data.network.portgroup)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Changing network portgroup is not supported));
-return -1;
+goto error;
 }
 
 if (!virNetDevVPortProfileEqual(oldnet-virtPortProfile,
 newnet-virtPortProfile)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Changing virtual port profile is not supported));
-return -1;
+goto error;
 }
 
 if (newnet-tune.sndbuf_specified) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Setting send buffer size is not supported));
-return -1;
+goto error;
 }
 
 if (!STREQ_NULLABLE(oldnet-script, newnet-script)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Setting startup script is not supported));
-return -1;
+goto error;
 }
 
 if (!STREQ_NULLABLE(oldnet-filter, newnet-filter)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Changing filter params is not supported));
-return -1;
+goto error;
 }
 
 if (newnet-bandwidth != NULL) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Setting bandwidth params is not supported));
-return -1;
+goto error;
 }
 
 for (i = 0; i  sizeof(newnet-vlan); i++) {
 if (((char *)newnet-vlan)[i] != 0) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Setting vlan params is not supported));
-return -1;
+goto error;
 }
 }
 
@@ -1905,15 +1906,22 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr 
pdom,
 is_changed = true;
 }
 
+if (create)
+VIR_FREE(oldnet);
+
 if (!create  !is_changed) {
 /* nothing changed - no need to run prlctl */
 return 0;
 }
 
 if (virCommandRun(cmd, NULL))
-return -1;
+goto error;
 
 return 0;
+error:
+if (create)
+VIR_FREE(oldnet);
+return -1;
 }
 
 static int
-- 
1.7.11.7

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


[libvirt] [PATCH 03/14] storage: Resolve resource leak using 'vol' buffer

2013-01-09 Thread John Ferlan
---
 src/storage/storage_backend_rbd.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index f5c6b0f..f916de1 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -329,16 +329,23 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr 
conn ATTRIBUTE_UNUSED,
 goto out_of_memory;
 
 vol-name = strdup(name);
-if (vol-name == NULL)
+if (vol-name == NULL) {
+VIR_FREE(vol);
 goto out_of_memory;
+}
 
-if (STREQ(vol-name, ))
+if (STREQ(vol-name, )) {
+VIR_FREE(vol-name);
+VIR_FREE(vol);
 break;
+}
 
 name += strlen(name) + 1;
 
-if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr)  0)
+if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr)  0) {
+virStorageVolDefFree(vol);
 goto cleanup;
+}
 
 pool-volumes.objs[pool-volumes.count++] = vol;
 }
-- 
1.7.11.7

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


[libvirt] [PATCH 00/14] Resolve some RESOURCE_LEAK errors found by Coverity

2013-01-09 Thread John Ferlan
This patch addresses some Error: RESOURCE_LEAK (CWE-404) errors found by
Coverity. There are 100+ remaining; however, many seem to be false positives
which will be addressed in a separate patch.

The esx_driver.c and hyperv_driver.c changes could be tagged as false
positives; however, the changes made will avoid the Coverity error since
the freeing of the resource marked isn't based soley on the result.

The phyp_driver.c changes in openSSHSession() and its callers were the most
involved since the code with now use connection_data-sock in order to track
the socket so as to allow it to be closed once done with it. Of interest in
this change is that phypExec() used connection_data-sock even though it
had never been initialized.

John Ferlan (14):
  phyp: Resolve some file descriptor leaks
  esx: Address resource leak found by Coverity
  storage: Resolve resource leak using 'vol' buffer
  util: Resolve resource leak for 'res' in virSetInherit error path.
  locking: Resolve resource leak with 'dir' on non error path
  rpc: Avoid resource leak of 'socks' if any object append fails
  uml: Avoid resource leak of event in umlInofityEvent
  test: Resource resource leak with 'tmp_vols'
  storage: Resource resource leak using 'tmp_vols'
  interface: Resolve resource leak wth 'tmp_iface_objs'
  xen: Resource resource leak with 'cpuset'
  parallels: Resolve some resource leaks
  storage: Need to also VIR_FREE(reg)
  hyperv: Address resource leak found by Coverityo

 src/esx/esx_driver.c|  8 
 src/hyperv/hyperv_driver.c  | 10 +-
 src/interface/interface_backend_netcf.c |  1 +
 src/locking/lock_driver_sanlock.c   |  1 +
 src/parallels/parallels_driver.c| 30 +++---
 src/phyp/phyp_driver.c  | 19 ++-
 src/rpc/virnetserverservice.c   |  6 +++---
 src/storage/storage_backend_logical.c   |  1 +
 src/storage/storage_backend_rbd.c   | 13 ++---
 src/storage/storage_driver.c|  1 +
 src/test/test_driver.c  |  1 +
 src/uml/uml_driver.c|  6 --
 src/util/virlockspace.c |  1 +
 src/xen/xend_internal.c | 12 ++--
 14 files changed, 71 insertions(+), 39 deletions(-)

-- 
1.7.11.7

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


[libvirt] [PATCH 02/14] esx: Address resource leak found by Coverity

2013-01-09 Thread John Ferlan
Because result was used to determine whether or not to free 'priv'
resources Coverity tagged the code as having a resource leak. This
change addresses that concern.
---
 src/esx/esx_driver.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 1366c81..9befa38 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -998,6 +998,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
 virReportOOMError();
 goto cleanup;
 }
+conn-privateData = priv;
 
 if (esxUtil_ParseUri(priv-parsedUri, conn-uri)  0) {
 goto cleanup;
@@ -1008,8 +1009,6 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
 priv-supportsLongMode = esxVI_Boolean_Undefined;
 priv-usedCpuTimeCounterId = -1;
 
-conn-privateData = priv;
-
 /*
  * Set the port dependent on the transport protocol if no port is
  * specified. This allows us to rely on the port parameter being
@@ -1104,9 +1103,10 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
 result = VIR_DRV_OPEN_SUCCESS;
 
   cleanup:
-if (result == VIR_DRV_OPEN_ERROR) {
+if (result == VIR_DRV_OPEN_ERROR)
+conn-privateData = NULL;
+if (priv  !conn-privateData)
 esxFreePrivate(priv);
-}
 
 VIR_FREE(potentialVCenterIpAddress);
 
-- 
1.7.11.7

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


[libvirt] [PATCH 01/14] phyp: Resolve some file descriptor leaks

2013-01-09 Thread John Ferlan
The phypUUIDTable_Push and phypUUIDTable_Pull leaked their fd's on normal
return.  I also noted that the Read function had a cut-n-paste error from
the write function on a couple of VIR_WARN's

The openSSHSession leaked the sock on the failure path.  Additionally that
turns into the internal_socket in the phypOpen code.  That was neither saved
nor closed on any path. So I used the connnection_data-sock field to save
the socket for eventual close. Of interest here is that phypExec used the
connection_data-sock field even though it had never been initialized.
---
 src/phyp/phyp_driver.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index f89871f..b74cab8 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -577,6 +577,7 @@ phypUUIDTable_Push(virConnectPtr conn)
 libssh2_channel_free(channel);
 channel = NULL;
 }
+VIR_FORCE_FCLOSE(fd);
 virBufferFreeAndReset(username);
 return 0;
 
@@ -665,7 +666,7 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
 int id;
 
 if ((fd = open(local_file, O_RDONLY)) == -1) {
-VIR_WARN(Unable to write information to local file.);
+VIR_WARN(Unable to read information from local file.);
 goto err;
 }
 
@@ -682,13 +683,13 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
 uuid_table-lpars[i]-id = id;
 } else {
 VIR_WARN
-(Unable to read from information to local file.);
+(Unable to read from information from local file.);
 goto err;
 }
 
 rc = read(fd, uuid_table-lpars[i]-uuid, VIR_UUID_BUFLEN);
 if (rc != VIR_UUID_BUFLEN) {
-VIR_WARN(Unable to read information to local file.);
+VIR_WARN(Unable to read information from local file.);
 goto err;
 }
 }
@@ -713,7 +714,7 @@ phypUUIDTable_Pull(virConnectPtr conn)
 struct stat fileinfo;
 char buffer[1024];
 int rc = 0;
-int fd;
+int fd = -1;
 int got = 0;
 int amount = 0;
 int total = 0;
@@ -820,6 +821,7 @@ err:
 libssh2_channel_free(channel);
 channel = NULL;
 }
+VIR_FORCE_CLOSE(fd);
 return -1;
 }
 
@@ -985,7 +987,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
 const char *hostname = conn-uri-server;
 char *username = NULL;
 char *password = NULL;
-int sock;
+int sock = -1;
 int rc;
 struct addrinfo *ai = NULL, *cur;
 struct addrinfo hints;
@@ -1135,6 +1137,7 @@ disconnect:
 libssh2_session_disconnect(session, Disconnecting...);
 libssh2_session_free(session);
 err:
+VIR_FORCE_CLOSE(sock);
 VIR_FREE(userhome);
 VIR_FREE(pubkey);
 VIR_FREE(pvtkey);
@@ -1191,6 +1194,7 @@ phypOpen(virConnectPtr conn,
 virReportOOMError();
 goto failure;
 }
+connection_data-sock = -1;
 
 if (conn-uri-path) {
 /* need to shift one byte in order to remove the first / of URI 
component */
@@ -1227,6 +1231,7 @@ phypOpen(virConnectPtr conn,
 }
 
 connection_data-session = session;
+connection_data-sock = internal_socket;
 
 uuid_table-nlpars = 0;
 uuid_table-lpars = NULL;
@@ -1270,6 +1275,8 @@ failure:
 libssh2_session_free(session);
 }
 
+if (connection_data)
+VIR_FORCE_CLOSE(connection_data-sock);
 VIR_FREE(connection_data);
 
 return VIR_DRV_OPEN_ERROR;
@@ -1289,6 +1296,8 @@ phypClose(virConnectPtr conn)
 phypUUIDTable_Free(phyp_driver-uuid_table);
 VIR_FREE(phyp_driver-managed_system);
 VIR_FREE(phyp_driver);
+
+VIR_FORCE_CLOSE(connection_data-sock);
 VIR_FREE(connection_data);
 return 0;
 }
-- 
1.7.11.7

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


[libvirt] [PATCH 04/14] util: Resolve resource leak for 'res' in virSetInherit error path.

2013-01-09 Thread John Ferlan
---
 src/util/virlockspace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 9ada6a6..04aeebe 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -387,6 +387,7 @@ virLockSpacePtr 
virLockSpaceNewPostExecRestart(virJSONValuePtr object)
 if (virSetInherit(res-fd, false)  0) {
 virReportSystemError(errno, %s,
  _(Cannot enable close-on-exec flag));
+virLockSpaceResourceFree(res);
 goto error;
 }
 if (virJSONValueObjectGetBoolean(child, lockHeld, res-lockHeld)  
0) {
-- 
1.7.11.7

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


Re: [libvirt] [PATCH qom-cpu 4/7] target-i386/cpu: Introduce FeatureWord typedefs

2013-01-09 Thread Gleb Natapov
On Mon, Jan 07, 2013 at 04:20:45PM -0200, Eduardo Habkost wrote:
 This introduces a FeatureWord enum, FeatureWordInfo struct (with
 generation information about a feature word), and a FeatureWordArray
 typedef, and changes add_flagname_to_bitmaps() code and
 cpu_x86_parse_featurestr() to use the new typedefs instead of separate
 variables for each feature word.
 
 This will help us keep the code at kvm_check_features_against_host(),
 cpu_x86_parse_featurestr() and add_flagname_to_bitmaps() sane while
 adding new feature name arrays.
 
 Signed-off-by: Eduardo Habkost ehabk...@redhat.com
 ---
  target-i386/cpu.c | 97 
 +++
  target-i386/cpu.h | 15 +
  2 files changed, 63 insertions(+), 49 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index b09b625..7d62d48 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -124,6 +124,20 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
  NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
  };
  
 +typedef struct FeatureWordInfo {
 +const char **feat_names;
 +} FeatureWordInfo;
 +
 +static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 +[FEAT_1_EDX] = { .feat_names = feature_name },
 +[FEAT_1_ECX] = { .feat_names = ext_feature_name },
 +[FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name },
 +[FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name },
 +[FEAT_KVM]   = { .feat_names = kvm_feature_name },
 +[FEAT_SVM]   = { .feat_names = svm_feature_name },
 +[FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name },
 +};
 +
  const char *get_register_name_32(unsigned int reg)
  {
  static const char *reg_names[CPU_NB_REGS32] = {
 @@ -271,23 +285,20 @@ static bool lookup_feature(uint32_t *pval, const char 
 *s, const char *e,
  return found;
  }
  
 -static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
 -uint32_t *ext_features,
 -uint32_t *ext2_features,
 -uint32_t *ext3_features,
 -uint32_t *kvm_features,
 -uint32_t *svm_features,
 -uint32_t *cpuid_7_0_ebx_features)
 +static void add_flagname_to_bitmaps(const char *flagname,
 +FeatureWordArray words)
  {
 -if (!lookup_feature(features, flagname, NULL, feature_name) 
 -!lookup_feature(ext_features, flagname, NULL, ext_feature_name) 
 -!lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) 
 -!lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) 
 -!lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) 
 -!lookup_feature(svm_features, flagname, NULL, svm_feature_name) 
 -!lookup_feature(cpuid_7_0_ebx_features, flagname, NULL,
 -cpuid_7_0_ebx_feature_name))
 -fprintf(stderr, CPU feature %s not found\n, flagname);
 +FeatureWord w;
 +for (w = 0; w  FEATURE_WORDS; w++) {
 +FeatureWordInfo *wi = feature_word_info[w];
 +if (wi-feat_names 
I would move patch 6 before that one and drop this check here, but if
you disagree it is your call.

 +lookup_feature(words[w], flagname, NULL, wi-feat_names)) {
 +break;
 +}
 +}
 +if (w == FEATURE_WORDS) {
 +fprintf(stderr, CPU feature %s not found\n, flagname);
 +}
  }
  
  typedef struct x86_def_t {
 @@ -1256,35 +1267,23 @@ static int cpu_x86_parse_featurestr(x86_def_t 
 *x86_cpu_def, char *features)
  unsigned int i;
  char *featurestr; /* Single 'key=value string being parsed */
  /* Features to be added */
 -uint32_t plus_features = 0, plus_ext_features = 0;
 -uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
 -uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
 -uint32_t plus_7_0_ebx_features = 0;
 +FeatureWordArray plus_features = {
 +[FEAT_KVM] = kvm_default_features,
 +};
  /* Features to be removed */
 -uint32_t minus_features = 0, minus_ext_features = 0;
 -uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
 -uint32_t minus_kvm_features = 0, minus_svm_features = 0;
 -uint32_t minus_7_0_ebx_features = 0;
 +FeatureWordArray minus_features = { 0 };
  uint32_t numvalue;
  
 -add_flagname_to_bitmaps(hypervisor, plus_features,
 -plus_ext_features, plus_ext2_features, plus_ext3_features,
 -plus_kvm_features, plus_svm_features,  plus_7_0_ebx_features);
 +add_flagname_to_bitmaps(hypervisor, plus_features);
  
  featurestr = features ? strtok(features, ,) : NULL;
  
  while (featurestr) {
  char *val;
  if (featurestr[0] == '+') {
 -add_flagname_to_bitmaps(featurestr + 1, plus_features,
 -

Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Eric Blake
On 01/09/2013 01:39 AM, Amos Kong wrote:
 Current seabios will try to boot from selected devices first,
 if they are all failed, seabios will also try to boot from
 un-selected devices.
 
 We need to make it configurable. I already posted a seabios
 patch to add a new device type to halt booting. Qemu can add
 HALT at the end of bootindex string, then seabios will halt
 booting after trying to boot from selected devices.
 
 This option only effects when boot priority is changed by
 bootindex options, the old style(-boot order=..) will still
 try to boot from un-selected devices.
 
 v2: add HALT entry in get_boot_devices_list()
 define boot_strict to bool
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---

Libvirt will need to expose an attribute that lets the user control
whether to use this new option; how do we probe via QMP whether the new
-boot strict=on command-line option is available?

 +++ b/qemu-options.hx
 @@ -376,14 +376,14 @@ ETEXI
  
  DEF(boot, HAS_ARG, QEMU_OPTION_boot,
  -boot [order=drives][,once=drives][,menu=on|off]\n
 -  
 [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time]\n
 +  
 [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n
  'drives': floppy (a), hard disk (c), CD-ROM (d), 
 network (n)\n
  'sp_name': the file's name that would be passed to bios 
 as logo picture, if menu=on\n
  'sp_time': the period that splash picture last if 
 menu=on, unit is ms\n
  'rb_timeout': the timeout before guest reboot when boot 
 failed, unit is ms\n,

So if I understand correctly, -boot order=... is incompatible with -boot
strict=on; even though you have listed both options under a single -boot
entry in the -help. We've already declared that -help output is no
longer guaranteed stable, so this doesn't really impact libvirt, but
would it make any more sense to list this as two orthogonal entries, to
make it clear that they don't mix?

-boot order=drivers[,once=drives]...
-boot strict=on|off[,menu=on|off]...

But this is all bikeshedding, so it's not worth a v3 if you disagree.

-- 
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 qom-cpu 0/7] disable kvm_mmu + -cpu enforce fixes (v3)

2013-01-09 Thread Gleb Natapov
On Mon, Jan 07, 2013 at 04:20:41PM -0200, Eduardo Habkost wrote:
 Changes on v3:
  - Patches 3-9 from v2 are now already on qom-cpu tree
  - Remove CONFIG_KVM #ifdefs by declaring fake KVM_* #defines on sysemu/kvm.h
  - Refactor code that uses the feature word arrays
(to make it easier to add a new feature name array)
  - Add feature name array for CPUID leaf 0xC001
 
 Changes on v2:
  - Now both the kvm_mmu-disable and -cpu enforce changes are on the same
series
  - Coding style fixes
 
 Git tree for reference:
   git://github.com/ehabkost/qemu-hacks.git cpu-enforce-all.v3
   https://github.com/ehabkost/qemu-hacks/tree/cpu-enforce-all.v3
 
 The changes are a bit intrusive, but:
 
  - The longer we take to make enforce strict as it should (and make libvirt
finally use it), more users will have VMs with migration-unsafe 
 unpredictable
guest ABIs. For this reason, I would like to get this into QEMU 1.4.
  - The changes in this series should affect only users that are already using
the enforce flag, and I believe whoever is using the enforce flag 
 really
want the strict behavior introduced by this series.
 
 
 
Reviewed-by: Gleb Natapov g...@redhat.com

Small comment on patch 4. Fill free to ignore.

 Eduardo Habkost (7):
   kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code
   target-i386: Don't set any KVM flag by default if KVM is disabled
   target-i386: Disable kvm_mmu by default
   target-i386/cpu: Introduce FeatureWord typedefs
   target-i386: kvm_check_features_against_host(): Use feature_word_info
   target-i386/cpu.c: Add feature name array for ext4_features
   target-i386: check/enforce: Check all feature words
 
  include/sysemu/kvm.h |  14 
  target-i386/cpu.c| 193 
 ---
  target-i386/cpu.h|  15 
  3 files changed, 150 insertions(+), 72 deletions(-)
 
 -- 
 1.7.11.7

--
Gleb.

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


Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
 On 01/09/2013 01:39 AM, Amos Kong wrote:
  Current seabios will try to boot from selected devices first,
  if they are all failed, seabios will also try to boot from
  un-selected devices.
  
  We need to make it configurable. I already posted a seabios
  patch to add a new device type to halt booting. Qemu can add
  HALT at the end of bootindex string, then seabios will halt
  booting after trying to boot from selected devices.
  
  This option only effects when boot priority is changed by
  bootindex options, the old style(-boot order=..) will still
  try to boot from un-selected devices.
  
  v2: add HALT entry in get_boot_devices_list()
  define boot_strict to bool
  
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
 
 Libvirt will need to expose an attribute that lets the user control
 whether to use this new option; how do we probe via QMP whether the new
 -boot strict=on command-line option is available?

While libvirt should make use of this, we don't need to
expose it in the XML. This new behaviour is what we wanted
to have all along, so we should just enable it.

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 2/2] build acpitable argument for qemu

2013-01-09 Thread Cole Robinson
On 01/09/2013 05:31 AM, Daniel P. Berrange wrote:
 On Wed, Jan 09, 2013 at 10:35:33AM +0800, liguang wrote:
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/conf/domain_conf.c  |1 +
  src/conf/domain_conf.h  |1 +
  src/qemu/qemu_command.c |4 
  3 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 6a7646e..54ba77f 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -9548,6 +9548,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
 caps,
  
  def-os.bootloader = virXPathString(string(./bootloader), ctxt);
  def-os.bootloaderArgs = virXPathString(string(./bootloader_args), 
 ctxt);
 +def-os.acpitable = virXPathString(string(./bootloader_args), ctxt);
  
 
 This is clearly bogus - you can't just grab an existing XML field and
 repurpose it. Second we shouldn't be requireing the user to specify
 custom ACPI tables just to use the machine type. libvirt should do the
 right thing with q35.
 
 Daniel
 

Also, there are patches on qemu-devel which claim to remove the -acpitable
requirement.

https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg00356.html

- Cole

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


Re: [libvirt] [PATCH 13/14] storage: Need to also VIR_FREE(reg)

2013-01-09 Thread Ján Tomko
On 01/09/13 15:54, John Ferlan wrote:
 Commit-id 'afc4631b' added the regfree(reg) to free resources alloc'd
 during regcomp; however, reg still needed to be VIR_FREE()'d.
 ---
  src/storage/storage_backend_logical.c | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/src/storage/storage_backend_logical.c 
 b/src/storage/storage_backend_logical.c
 index bd902fe..38b9f08 100644
 --- a/src/storage/storage_backend_logical.c
 +++ b/src/storage/storage_backend_logical.c
 @@ -265,6 +265,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
  cleanup:
  VIR_FREE(regex);
  regfree(reg);
 +VIR_FREE(reg);
  VIR_FREE(vars);
  if (is_new_vol  (ret == -1))
  virStorageVolDefFree(vol);

Eww, who did that? O:-)

Now I see it's not the only thing wrong with my fix, since regfree
doesn't accept NULL.

Jan

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


Re: [libvirt] Connection release is not correct in libvirt and libvrt java

2013-01-09 Thread Claudio Bley
Hi, sorry for replying so late.

At Wed, 19 Dec 2012 15:31:54 -0700,
Eric Blake wrote:
 
 On 12/18/2012 08:14 AM, Benjamin Wang (gendwang) wrote:
  For libvirt java, there are similar issue. I have changed code as following 
  in Collect.java. Please also give your comments.
  public int close() throws LibvirtException {
  int success = 0;
  if (VCP != null) {
  +try {
   success = libvirt.virConnectClose(VCP);
  processError();
  +}
  +finally {
  // If leave an invalid pointer dangling around JVM crashes 
  and burns
  // if someone tries to call a method on us
  // We rely on the underlying libvirt error handling to 
  detect that
  // it's called with a null virConnectPointer
  VCP = null;
   +   }
 
 Unfortunately, I'm not familiar enough with libvirt-java to know if this
 patch makes sense.

Basically, when applying this patch it gives you only a one-chance
opportunity to call virConnectClose.

If an exception is thrown, it will set VCP to null, no matter what.

Might be that there was only a temporary problem, and you could
retry... ? Or do some other stuff if the VCP is still valid. But that
depends on the error.

So, is it ensured, that when virConnectClose returns an error, the
virConnectPtr is wasted and should be invalidated?

Claudio
-- 
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:http://www.av-test.org

Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

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

Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Amos Kong
On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
 On 01/09/2013 01:39 AM, Amos Kong wrote:
  Current seabios will try to boot from selected devices first,
  if they are all failed, seabios will also try to boot from
  un-selected devices.
  
  We need to make it configurable. I already posted a seabios
  patch to add a new device type to halt booting. Qemu can add
  HALT at the end of bootindex string, then seabios will halt
  booting after trying to boot from selected devices.
  
  This option only effects when boot priority is changed by
  bootindex options, the old style(-boot order=..) will still
  try to boot from un-selected devices.
  
  v2: add HALT entry in get_boot_devices_list()
  define boot_strict to bool
  
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
 
 Libvirt will need to expose an attribute that lets the user control
 whether to use this new option; how do we probe via QMP whether the new
 -boot strict=on command-line option is available?


Old style to adjust boot priority by order parameter:
-boot order=n,strict=on (BAD, unselected devices will be tried)

New style to adjust boot priority by bootindex:
-device virtio-net-pci,...,bootindex=1 -boot strict=on (OK)

We only want strict option to support new style.

(those two styles are implemented in two different way insider
seabios, the latest simple patch only changed the bootindex way)
 
  +++ b/qemu-options.hx
  @@ -376,14 +376,14 @@ ETEXI
   
   DEF(boot, HAS_ARG, QEMU_OPTION_boot,
   -boot [order=drives][,once=drives][,menu=on|off]\n
  -  
  [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time]\n
  +  
  [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n
   'drives': floppy (a), hard disk (c), CD-ROM (d), 
  network (n)\n
   'sp_name': the file's name that would be passed to 
  bios as logo picture, if menu=on\n
   'sp_time': the period that splash picture last if 
  menu=on, unit is ms\n
   'rb_timeout': the timeout before guest reboot when 
  boot failed, unit is ms\n,
 
 So if I understand correctly, -boot order=... is incompatible with -boot
 strict=on; even though you have listed both options under a single -boot
 entry in the -help. We've already declared that -help output is no
 longer guaranteed stable, so this doesn't really impact libvirt, but
 would it make any more sense to list this as two orthogonal entries, to
 make it clear that they don't mix?
 
 -boot order=drivers[,once=drives]...
 -boot strict=on|off[,menu=on|off]...
 
 But this is all bikeshedding, so it's not worth a v3 if you disagree.
 
 -- 
 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] Make TLS support conditional

2013-01-09 Thread Eric Blake
On 01/09/2013 02:48 AM, Daniel P. Berrange wrote:
 As it is, with your patch, I just got this failure on RHEL 5:

 /usr/bin/perl ./check-symfile.pl l ibvirt.yms \
 .libs/libvirt.so
 Expected symbol virNetServerClientGetTLSKeySize is not in ELF library
 ...

 I still need to do more investigation, but it makes me wonder if we got
 the conditional symfile manipulation correct?
 
 Yeah, actually I think that's something I forgot to handle.

We still need to fix that for when a user explicitly configures
--without-gnutls.

 on RHEL5, GNUTLS should be present so that symbol ought to have been
 built, unless you were testing with --without-gnutls perhaps ?

Figured it out: on RHEL 5, automake and autoconf are so old that they
don't automatically reconfigure when you use just 'make', so config.h
was out of date, and the new HAVE_GNUTLS macro wasn't defined.  Forcing
a fresh reconfigure fixed the issue to use gnutls in a default build.

-- 
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] virsh: add --start option to the define command

2013-01-09 Thread Eric Blake
On 01/09/2013 03:28 AM, Daniel P. Berrange wrote:

 We have virDomainDefineXML with no flags, but we have virDomainCreateXML
 with flags; maybe the better approach is to add a new creation flag that
 says that in addition to starting the domain, we also make it persistent
 at the same time.

 But if we do that, it would argue that 'virsh create --persistent
 blah.xml' is nicer than 'virsh define --start blah.xml', at least in
 that the former needs only 1 API call for new libvirt (but falls back to
 2 API calls when talking to older libvirt), while the latter always
 needs 2 API calls.

 Or maybe it means we need to add virDomainDefineXMLFlags().

 Anyone else want to throw some paint on the bikeshed on how best to make
 the user experience nicer?
 
 While I think the virsh idea is fine, I don't want to see this done
 at the virDomainDefine API level, since it just duplicates functionality
 already present. Just have virsh make an API call to the existing create
 API.

I agree that we don't really need to add virDomainDefineXMLFlags().  But
I'm not quite clear on your stance on adding a new flag to the existing
virDomainCreateXML().  A single API call is always nicer than two API
calls for atomicity reasons.  That is, I'm proposing that we add a new
flag, and that:

virDomainCreateXML(conn, xml, VIR_DOMAIN_CREATE_PERSISTENT)

be used to both start and define a domain in one call, and only if that
flag is unrecognized do we fall back to the two-API sequence of

virDomainCreateXML(conn, xml, 0)  virDomainDefineXML(conn, xml)

or the equivalent two-API sequence of

dom=virDomainDefineXML(conn, xml)  virDomainCreate(dom)

-- 
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 11/14] xen: Resource resource leak with 'cpuset'

2013-01-09 Thread Laine Stump
(you duplicated resource in the subject line)

On 01/09/2013 09:54 AM, John Ferlan wrote:
 Make cpuset local to the while loop and free it once done with it each
 time through the loop.
 ---
  src/xen/xend_internal.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index 84a25e8..c6b800b 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -1113,7 +1113,6 @@ sexpr_to_xend_topology(const struct sexpr *root,
  {
  const char *nodeToCpu;
  const char *cur;
 -virBitmapPtr cpuset = NULL;
  int *cpuNums = NULL;
  int cell, cpu, nb_cpus;
  int n = 0;
 @@ -1131,6 +1130,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
  
  cur = nodeToCpu;
  while (*cur != 0) {
 +virBitmapPtr cpuset = NULL;
  /*
   * Find the next NUMA cell described in the xend output
   */
 @@ -1152,8 +1152,10 @@ sexpr_to_xend_topology(const struct sexpr *root,
  goto memory_error;
  } else {
  nb_cpus = virBitmapParse(cur, 'n', cpuset, numCpus);
 -if (nb_cpus  0)
 +if (nb_cpus  0) {
 +virBitmapFree(cpuset);

This virBitmapFree() isn't necessary - virBitmapParse is guaranteed to
have nothing allocated (and will set cpuset = NULL) if it fails.

  goto error;
 +}
  }
  
  for (n = 0, cpu = 0; cpu  numCpus; cpu++) {
 @@ -1163,28 +1165,26 @@ sexpr_to_xend_topology(const struct sexpr *root,
  if (used)
  cpuNums[n++] = cpu;
  }
 +virBitmapFree(cpuset);
  
  if (virCapabilitiesAddHostNUMACell(caps,
 cell,
 nb_cpus,
 cpuNums)  0)
  goto memory_error;
 +
  }
  VIR_FREE(cpuNums);
 -virBitmapFree(cpuset);
  return 0;
  
parse_error:
  virReportError(VIR_ERR_XEN_CALL, %s, _(topology syntax error));
error:
  VIR_FREE(cpuNums);
 -virBitmapFree(cpuset);
 -
  return -1;
  
memory_error:
  VIR_FREE(cpuNums);
 -virBitmapFree(cpuset);
  virReportOOMError();
  return -1;
  }

ACK with the above nits fixed.

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


Re: [libvirt] [PATCH] virsh: add --start option to the define command

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 09:49:31AM -0700, Eric Blake wrote:
 On 01/09/2013 03:28 AM, Daniel P. Berrange wrote:
 
  We have virDomainDefineXML with no flags, but we have virDomainCreateXML
  with flags; maybe the better approach is to add a new creation flag that
  says that in addition to starting the domain, we also make it persistent
  at the same time.
 
  But if we do that, it would argue that 'virsh create --persistent
  blah.xml' is nicer than 'virsh define --start blah.xml', at least in
  that the former needs only 1 API call for new libvirt (but falls back to
  2 API calls when talking to older libvirt), while the latter always
  needs 2 API calls.
 
  Or maybe it means we need to add virDomainDefineXMLFlags().
 
  Anyone else want to throw some paint on the bikeshed on how best to make
  the user experience nicer?
  
  While I think the virsh idea is fine, I don't want to see this done
  at the virDomainDefine API level, since it just duplicates functionality
  already present. Just have virsh make an API call to the existing create
  API.
 
 I agree that we don't really need to add virDomainDefineXMLFlags().  But
 I'm not quite clear on your stance on adding a new flag to the existing
 virDomainCreateXML().  A single API call is always nicer than two API
 calls for atomicity reasons.  That is, I'm proposing that we add a new
 flag, and that:
 
 virDomainCreateXML(conn, xml, VIR_DOMAIN_CREATE_PERSISTENT)

I don't really like this either. I think it is good that the APIs
are separate because it gives applications greater clarity on
error handling. eg with this API you could get an case where
the API defines the XML, starts the VM, fails and then tries
to undefine the XML but fails that too. Now the caller sees an
error, but can't know whether the XML was undefined or not.
If the app does the separate API calls they have full control
over what happens in that situation

 be used to both start and define a domain in one call, and only if that
 flag is unrecognized do we fall back to the two-API sequence of
 
 virDomainCreateXML(conn, xml, 0)  virDomainDefineXML(conn, xml)
 
 or the equivalent two-API sequence of
 
 dom=virDomainDefineXML(conn, xml)  virDomainCreate(dom)

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 PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Laine Stump
On 01/09/2013 10:22 AM, Daniel P. Berrange wrote:
 On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
 On 01/09/2013 01:39 AM, Amos Kong wrote:
 Current seabios will try to boot from selected devices first,
 if they are all failed, seabios will also try to boot from
 un-selected devices.

 We need to make it configurable. I already posted a seabios
 patch to add a new device type to halt booting. Qemu can add
 HALT at the end of bootindex string, then seabios will halt
 booting after trying to boot from selected devices.

 This option only effects when boot priority is changed by
 bootindex options, the old style(-boot order=..) will still
 try to boot from un-selected devices.

 v2: add HALT entry in get_boot_devices_list()
 define boot_strict to bool

 Signed-off-by: Amos Kong ak...@redhat.com
 ---
 Libvirt will need to expose an attribute that lets the user control
 whether to use this new option; how do we probe via QMP whether the new
 -boot strict=on command-line option is available?
 While libvirt should make use of this, we don't need to
 expose it in the XML. This new behaviour is what we wanted
 to have all along, so we should just enable it.

I agree that this is the way it *should* always work, but apparently
there are people who depend on the old behavior, so just doing a blanket
switch to the new behavior could lead to setups that no longer work
properly after an upgrade, which unfortunately means that existing
functionality needs to be maintained, and correct functionality must
be triggered by a config switch (maybe Gleb can expand on the use cases
that require this if more details are needed, as it's him I heard this from)

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


Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Laine Stump
On 01/09/2013 10:52 AM, Amos Kong wrote:
 On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
 On 01/09/2013 01:39 AM, Amos Kong wrote:
 Current seabios will try to boot from selected devices first,
 if they are all failed, seabios will also try to boot from
 un-selected devices.

 We need to make it configurable. I already posted a seabios
 patch to add a new device type to halt booting. Qemu can add
 HALT at the end of bootindex string, then seabios will halt
 booting after trying to boot from selected devices.

 This option only effects when boot priority is changed by
 bootindex options, the old style(-boot order=..) will still
 try to boot from un-selected devices.

 v2: add HALT entry in get_boot_devices_list()
 define boot_strict to bool

 Signed-off-by: Amos Kong ak...@redhat.com
 ---
 Libvirt will need to expose an attribute that lets the user control
 whether to use this new option; how do we probe via QMP whether the new
 -boot strict=on command-line option is available?

 Old style to adjust boot priority by order parameter:
 -boot order=n,strict=on (BAD, unselected devices will be tried)

 New style to adjust boot priority by bootindex:
 -device virtio-net-pci,...,bootindex=1 -boot strict=on (OK)

 We only want strict option to support new style.

 (those two styles are implemented in two different way insider
 seabios, the latest simple patch only changed the bootindex way)

Just a note about this: as far as I can tell virt-manager currently only
uses the old style of specifying boot order; it needs to be enhanced
to recognize the presence of bootindex=n ordering, and behave
accordingly. Looking from the outside, that looks to be not completely
trivial, as the old style of ordering only allows specifying hard disk
as a single line in the priority order, but the new style has each disk
specified separately. Not only that, but *which* of the disks is tried
first under the old order may change depending on whether or not a
bootmenu is requested (in one case it picks unit=0 on the controller, in
the other case, it orders the disks alphabetically by target dev name)


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


Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Gleb Natapov
On Wed, Jan 09, 2013 at 12:28:57PM -0500, Laine Stump wrote:
 On 01/09/2013 10:22 AM, Daniel P. Berrange wrote:
  On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
  On 01/09/2013 01:39 AM, Amos Kong wrote:
  Current seabios will try to boot from selected devices first,
  if they are all failed, seabios will also try to boot from
  un-selected devices.
 
  We need to make it configurable. I already posted a seabios
  patch to add a new device type to halt booting. Qemu can add
  HALT at the end of bootindex string, then seabios will halt
  booting after trying to boot from selected devices.
 
  This option only effects when boot priority is changed by
  bootindex options, the old style(-boot order=..) will still
  try to boot from un-selected devices.
 
  v2: add HALT entry in get_boot_devices_list()
  define boot_strict to bool
 
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
  Libvirt will need to expose an attribute that lets the user control
  whether to use this new option; how do we probe via QMP whether the new
  -boot strict=on command-line option is available?
  While libvirt should make use of this, we don't need to
  expose it in the XML. This new behaviour is what we wanted
  to have all along, so we should just enable it.
 
 I agree that this is the way it *should* always work, but apparently
 there are people who depend on the old behavior, so just doing a blanket
 switch to the new behavior could lead to setups that no longer work
 properly after an upgrade, which unfortunately means that existing
 functionality needs to be maintained, and correct functionality must
 be triggered by a config switch (maybe Gleb can expand on the use cases
 that require this if more details are needed, as it's him I heard this from)

It is common to configure PXE boot as highest prio for easy re-imaging,
Usually boot from PXE fails and other bootable device is used instead,
but if re-installation is needed it is as easy as configuring PXE server
and rebooting the machine. With current behaviour it is enough to boost
PXE priority, if we will change it setups that didn't explicitly specify
all devices' priorities will stop working. The rule is easy: if exist
command line that will work differently before and after the change you
probably break someones setup with your patch.

--
Gleb.

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


Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Daniel P. Berrange
On Wed, Jan 09, 2013 at 12:36:52PM -0500, Laine Stump wrote:
 On 01/09/2013 10:52 AM, Amos Kong wrote:
  On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
  On 01/09/2013 01:39 AM, Amos Kong wrote:
  Current seabios will try to boot from selected devices first,
  if they are all failed, seabios will also try to boot from
  un-selected devices.
 
  We need to make it configurable. I already posted a seabios
  patch to add a new device type to halt booting. Qemu can add
  HALT at the end of bootindex string, then seabios will halt
  booting after trying to boot from selected devices.
 
  This option only effects when boot priority is changed by
  bootindex options, the old style(-boot order=..) will still
  try to boot from un-selected devices.
 
  v2: add HALT entry in get_boot_devices_list()
  define boot_strict to bool
 
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
  Libvirt will need to expose an attribute that lets the user control
  whether to use this new option; how do we probe via QMP whether the new
  -boot strict=on command-line option is available?
 
  Old style to adjust boot priority by order parameter:
  -boot order=n,strict=on (BAD, unselected devices will be tried)
 
  New style to adjust boot priority by bootindex:
  -device virtio-net-pci,...,bootindex=1 -boot strict=on (OK)
 
  We only want strict option to support new style.
 
  (those two styles are implemented in two different way insider
  seabios, the latest simple patch only changed the bootindex way)
 
 Just a note about this: as far as I can tell virt-manager currently only
 uses the old style of specifying boot order; it needs to be enhanced
 to recognize the presence of bootindex=n ordering, and behave
 accordingly. Looking from the outside, that looks to be not completely
 trivial, as the old style of ordering only allows specifying hard disk
 as a single line in the priority order, but the new style has each disk
 specified separately. Not only that, but *which* of the disks is tried
 first under the old order may change depending on whether or not a
 bootmenu is requested (in one case it picks unit=0 on the controller, in
 the other case, it orders the disks alphabetically by target dev name)

Hmm, well libvirt should be using bootindex=n on the QEMU command line
regardless of what applications put in the XML. ie if the application
uses the old style XML, libvirt should translate that into bootindex=n
for them.


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 11/14] xen: Resource resource leak with 'cpuset'

2013-01-09 Thread John Ferlan
On 01/09/2013 11:55 AM, Laine Stump wrote:
 (you duplicated resource in the subject line)
 

Missed that one... Will fix.

 On 01/09/2013 09:54 AM, John Ferlan wrote:
 Make cpuset local to the while loop and free it once done with it each
 time through the loop.
 ---
  src/xen/xend_internal.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index 84a25e8..c6b800b 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -1113,7 +1113,6 @@ sexpr_to_xend_topology(const struct sexpr *root,
  {
  const char *nodeToCpu;
  const char *cur;
 -virBitmapPtr cpuset = NULL;
  int *cpuNums = NULL;
  int cell, cpu, nb_cpus;
  int n = 0;
 @@ -1131,6 +1130,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
  
  cur = nodeToCpu;
  while (*cur != 0) {
 +virBitmapPtr cpuset = NULL;
  /*
   * Find the next NUMA cell described in the xend output
   */
 @@ -1152,8 +1152,10 @@ sexpr_to_xend_topology(const struct sexpr *root,
  goto memory_error;
  } else {
  nb_cpus = virBitmapParse(cur, 'n', cpuset, numCpus);
 -if (nb_cpus  0)
 +if (nb_cpus  0) {
 +virBitmapFree(cpuset);
 
 This virBitmapFree() isn't necessary - virBitmapParse is guaranteed to
 have nothing allocated (and will set cpuset = NULL) if it fails.
 

According to Coverity's analysis this may not be true since it's
possible to hit the ret-- line (more than once) in virBitmapParse()
while hitting either ret++ line less times returning a negative value
on the success path. The example Coverity had shows 6 passes through
the loop, 4 negatives, 1 positive, and 1 nothing.

Whether realistically this could be true, I am not sure.

How Coverity determined what the value of 'cpuSet' is a mystery as the
output I have doesn't show what's being used for parsing, just that we
go through the loop 6 times. Perhaps something like ^1,^2,^3,4,^5,^6
where 1,2,3,4,5,6 pass the virBitmapIsSet() call changing the 'ret'
value to -3.


  goto error;
 +}
  }
  
  for (n = 0, cpu = 0; cpu  numCpus; cpu++) {
 @@ -1163,28 +1165,26 @@ sexpr_to_xend_topology(const struct sexpr *root,
  if (used)
  cpuNums[n++] = cpu;
  }
 +virBitmapFree(cpuset);
  
  if (virCapabilitiesAddHostNUMACell(caps,
 cell,
 nb_cpus,
 cpuNums)  0)
  goto memory_error;
 +
  }
  VIR_FREE(cpuNums);
 -virBitmapFree(cpuset);
  return 0;
  
parse_error:
  virReportError(VIR_ERR_XEN_CALL, %s, _(topology syntax error));
error:
  VIR_FREE(cpuNums);
 -virBitmapFree(cpuset);
 -
  return -1;
  
memory_error:
  VIR_FREE(cpuNums);
 -virBitmapFree(cpuset);
  virReportOOMError();
  return -1;
  }
 
 ACK with the above nits fixed.
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list
 

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


[libvirt] [libvirt-glib] gobject: API to open read-only connection to libvirt

2013-01-09 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

---
 libvirt-gobject/libvirt-gobject-connection.c | 99 +---
 libvirt-gobject/libvirt-gobject-connection.h | 10 +++
 libvirt-gobject/libvirt-gobject.sym  |  7 ++
 3 files changed, 107 insertions(+), 9 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 3157a66..558ed2a 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -407,14 +407,10 @@ static int domain_event_cb(virConnectPtr conn 
G_GNUC_UNUSED,
 return 0;
 }
 
-/**
- * gvir_connection_open:
- * @conn: a #GVirConnection
- * @cancellable: (allow-none)(transfer none): cancellation object
- */
-gboolean gvir_connection_open(GVirConnection *conn,
-  GCancellable *cancellable,
-  GError **err)
+static gboolean _gvir_connection_open(GVirConnection *conn,
+  gboolean read_only,
+  GCancellable *cancellable,
+  GError **err)
 {
 GVirConnectionPrivate *priv;
 
@@ -438,7 +434,11 @@ gboolean gvir_connection_open(GVirConnection *conn,
 return FALSE;
 }
 
-if (!(priv-conn = virConnectOpen(priv-uri))) {
+if (read_only)
+priv-conn = virConnectOpenReadOnly(priv-uri);
+else
+priv-conn = virConnectOpen(priv-uri);
+if (!priv-conn) {
 gvir_set_error(err, GVIR_CONNECTION_ERROR,
0,
Unable to open %s,
@@ -472,6 +472,24 @@ gboolean gvir_connection_open(GVirConnection *conn,
 return TRUE;
 }
 
+/**
+ * gvir_connection_open:
+ * @conn: a #GVirConnection
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ */
+gboolean gvir_connection_open(GVirConnection *conn,
+  GCancellable *cancellable,
+  GError **err)
+{
+return _gvir_connection_open(conn, FALSE, cancellable, err);
+}
+
+gboolean gvir_connection_open_read_only(GVirConnection *conn,
+GCancellable *cancellable,
+GError **err)
+{
+return _gvir_connection_open(conn, TRUE, cancellable, err);
+}
 
 static void
 gvir_connection_open_helper(GSimpleAsyncResult *res,
@@ -537,6 +555,69 @@ gboolean gvir_connection_open_finish(GVirConnection *conn,
 return TRUE;
 }
 
+static void
+gvir_connection_open_read_only_helper(GSimpleAsyncResult *res,
+GObject *object,
+GCancellable *cancellable)
+{
+GVirConnection *conn = GVIR_CONNECTION(object);
+GError *err = NULL;
+
+if (!gvir_connection_open_read_only(conn, cancellable, err)) {
+g_simple_async_result_set_from_error(res, err);
+g_error_free(err);
+}
+}
+
+
+/**
+ * gvir_connection_open_read_only_async:
+ * @conn: a #GVirConnection
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ */
+void gvir_connection_open_read_only_async(GVirConnection *conn,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+GSimpleAsyncResult *res;
+
+g_return_if_fail(GVIR_IS_CONNECTION(conn));
+g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
+
+res = g_simple_async_result_new(G_OBJECT(conn),
+callback,
+user_data,
+gvir_connection_open_read_only_async);
+g_simple_async_result_run_in_thread(res,
+gvir_connection_open_read_only_helper,
+G_PRIORITY_DEFAULT,
+cancellable);
+g_object_unref(res);
+}
+
+
+/**
+ * gvir_connection_open_read_only_finish:
+ * @conn: a #GVirConnection
+ * @result: (transfer none): async method result
+ */
+gboolean gvir_connection_open_read_only_finish(GVirConnection *conn,
+   GAsyncResult *result,
+   GError **err)
+{
+g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE);
+g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(conn),
+
gvir_connection_open_read_only_async),
+ FALSE);
+
+if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), 
err))
+return FALSE;
+
+return TRUE;
+}
 
 gboolean gvir_connection_is_open(GVirConnection *conn)
 {
diff --git a/libvirt-gobject/libvirt-gobject-connection.h 

[libvirt] Slow libvirtd startup

2013-01-09 Thread Zeeshan Ali (Khattak)
Hi,
  Too lazy to explain again so I'll just cp conversation I had about
the issue with Eric on IRC:

zeenix is timeouts on session libvirt (or in general) something new?
zeenix libvirtd startup seems to now take a few seconds so Boxes UI
is all empty for that amount of time
eblake it's been around for a while; qemu.conf has settings to
control how long it should take
zeenix unless Boxes is launched before libvirtd timesout
eblake but there is a relatively new patch that makes the libvirtd
-t actually be useful for qemu:///session
zeenix don't think i ever changed that, at least not recently
zeenix that could be it
zeenix i only saw it happening after i update libvirtd (after a month)
eblake if you're seeing some delays in starting libvirtd, those are
probably worth fixing
zeenix s/update/recently updated/
zeenix yeah
zeenix i'll try to write a test app
zeenix no need, i can reproduce with virsh :)

[zeenix@z-laptop virt]$ time virsh list
 IdName   State



real0m6.349s
user0m0.016s
sys 0m0.010s
[zeenix@z-laptop virt]$ time virsh list
 IdName   State



real0m0.024s
user0m0.010s
sys 0m0.011s


-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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


[libvirt] [PATCH] maint: distribute libvirtd.service.in

2013-01-09 Thread Eric Blake
I did a build --without-libvirtd, then ran 'make dist'.  The
resulting tarball was broken, with a complaint that make did not
know how to create libvirtd.service.in.  I traced it to a use
of EXTRA_DIST inside a conditional.

* daemon/Makefile.am (EXTRA_DIST): Hoist libvirtd.service.in
outside of WITH_LIBVIRTD conditional.
---

Pushing under the build-breaker rule.

 daemon/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index c59084c..95ff8cf 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -37,6 +37,7 @@ EXTRA_DIST =  \
libvirtd.upstart\
libvirtd.policy.in  \
libvirtd.sasl   \
+   libvirtd.service.in \
libvirtd.sysconf\
libvirtd.sysctl \
libvirtd.aug\
@@ -322,7 +323,6 @@ uninstall-init-upstart:
 endif # LIBVIRT_INIT_SCRIPT_UPSTART


-EXTRA_DIST += libvirtd.service.in
 if LIBVIRT_INIT_SCRIPT_SYSTEMD

 SYSTEMD_UNIT_DIR = /lib/systemd/system
-- 
1.8.0.2

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


[libvirt] [libvirt-glib v2] gobject: API to open read-only connection to

2013-01-09 Thread Zeeshan Ali (Khattak)
* Corrected tabs in .syms entry
* Added a utility method to check if connection is open in read-only mode.

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


[libvirt] [libvirt-glib v2] gobject: API to open read-only connection to libvirt

2013-01-09 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

Also a utility method to check if connection is open in read-only mode.
---
 libvirt-gobject/libvirt-gobject-connection.c | 109 ---
 libvirt-gobject/libvirt-gobject-connection.h |  11 +++
 libvirt-gobject/libvirt-gobject.sym  |   8 ++
 3 files changed, 119 insertions(+), 9 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 3157a66..dbbf136 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -38,6 +38,7 @@ struct _GVirConnectionPrivate
 GMutex *lock;
 gchar *uri;
 virConnectPtr conn;
+gboolean read_only;
 
 GHashTable *domains;
 GHashTable *pools;
@@ -407,14 +408,10 @@ static int domain_event_cb(virConnectPtr conn 
G_GNUC_UNUSED,
 return 0;
 }
 
-/**
- * gvir_connection_open:
- * @conn: a #GVirConnection
- * @cancellable: (allow-none)(transfer none): cancellation object
- */
-gboolean gvir_connection_open(GVirConnection *conn,
-  GCancellable *cancellable,
-  GError **err)
+static gboolean _gvir_connection_open(GVirConnection *conn,
+  gboolean read_only,
+  GCancellable *cancellable,
+  GError **err)
 {
 GVirConnectionPrivate *priv;
 
@@ -438,7 +435,13 @@ gboolean gvir_connection_open(GVirConnection *conn,
 return FALSE;
 }
 
-if (!(priv-conn = virConnectOpen(priv-uri))) {
+if (read_only) {
+priv-conn = virConnectOpenReadOnly(priv-uri);
+priv-read_only = TRUE;
+} else {
+priv-conn = virConnectOpen(priv-uri);
+}
+if (!priv-conn) {
 gvir_set_error(err, GVIR_CONNECTION_ERROR,
0,
Unable to open %s,
@@ -472,6 +475,24 @@ gboolean gvir_connection_open(GVirConnection *conn,
 return TRUE;
 }
 
+/**
+ * gvir_connection_open:
+ * @conn: a #GVirConnection
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ */
+gboolean gvir_connection_open(GVirConnection *conn,
+  GCancellable *cancellable,
+  GError **err)
+{
+return _gvir_connection_open(conn, FALSE, cancellable, err);
+}
+
+gboolean gvir_connection_open_read_only(GVirConnection *conn,
+GCancellable *cancellable,
+GError **err)
+{
+return _gvir_connection_open(conn, TRUE, cancellable, err);
+}
 
 static void
 gvir_connection_open_helper(GSimpleAsyncResult *res,
@@ -537,6 +558,69 @@ gboolean gvir_connection_open_finish(GVirConnection *conn,
 return TRUE;
 }
 
+static void
+gvir_connection_open_read_only_helper(GSimpleAsyncResult *res,
+GObject *object,
+GCancellable *cancellable)
+{
+GVirConnection *conn = GVIR_CONNECTION(object);
+GError *err = NULL;
+
+if (!gvir_connection_open_read_only(conn, cancellable, err)) {
+g_simple_async_result_set_from_error(res, err);
+g_error_free(err);
+}
+}
+
+
+/**
+ * gvir_connection_open_read_only_async:
+ * @conn: a #GVirConnection
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ */
+void gvir_connection_open_read_only_async(GVirConnection *conn,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+GSimpleAsyncResult *res;
+
+g_return_if_fail(GVIR_IS_CONNECTION(conn));
+g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
+
+res = g_simple_async_result_new(G_OBJECT(conn),
+callback,
+user_data,
+gvir_connection_open_read_only_async);
+g_simple_async_result_run_in_thread(res,
+gvir_connection_open_read_only_helper,
+G_PRIORITY_DEFAULT,
+cancellable);
+g_object_unref(res);
+}
+
+
+/**
+ * gvir_connection_open_read_only_finish:
+ * @conn: a #GVirConnection
+ * @result: (transfer none): async method result
+ */
+gboolean gvir_connection_open_read_only_finish(GVirConnection *conn,
+   GAsyncResult *result,
+   GError **err)
+{
+g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE);
+g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(conn),
+
gvir_connection_open_read_only_async),
+  

[libvirt] [PATCH] util: reduce syscalls for virGetDeviceID

2013-01-09 Thread Eric Blake
There's no need to do lots of readlink() calls to canonicalize
a name if we're only going to use stat() on it, since stat()
already chases symlinks.

* src/util/virutil.c (virGetDeviceID): Let stat() do the symlink
chasing.
---
 src/util/virutil.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 47ab17f..7c54bea 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1,7 +1,7 @@
 /*
  * virutil.c: common, generic utility functions
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  * Copyright (C) 2006, 2007 Binary Karma
  * Copyright (C) 2006 Shuveb Hussain
@@ -3135,27 +3135,18 @@ int
 virGetDeviceID(const char *path, int *maj, int *min)
 {
 struct stat sb;
-char *canonical_path = NULL;

-if (virFileResolveLink(path, canonical_path)  0)
+if (stat(path, sb)  0)
 return -errno;

-if (stat(canonical_path, sb)  0) {
-VIR_FREE(canonical_path);
-return -errno;
-}
-
-if (!S_ISBLK(sb.st_mode)) {
-VIR_FREE(canonical_path);
+if (!S_ISBLK(sb.st_mode))
 return -EINVAL;
-}

 if (maj)
 *maj = major(sb.st_rdev);
 if (min)
 *min = minor(sb.st_rdev);

-VIR_FREE(canonical_path);
 return 0;
 }
 #else
-- 
1.8.0.2

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


[libvirt] [PATCH] maint: fix comment typo

2013-01-09 Thread Eric Blake
While OOM can have knock-on effects that trash a system, generally
the first symptom is one of memory thrashing.

* src/qemu/qemu_cgroup.c (qemuSetupCgroup): Reword slightly.
---

Pushing under the trivial rule.

 src/qemu/qemu_cgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 16a9d7c..6527146 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1,7 +1,7 @@
 /*
  * qemu_cgroup.c: QEMU cgroup management
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -344,8 +344,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,

 if (!hard_limit) {
 /* If there is no hard_limit set, set a reasonable one to avoid
- * system trashing caused by exploited qemu.  As 'reasonable limit'
- * has been chosen:
+ * system thrashing caused by exploited qemu.  A 'reasonable
+ * limit' has been chosen:
  * (1 + k) * (domain memory + total video memory) + (32MB for
  * cache per each disk) + F
  * where k = 0.5 and F = 200MB.  The cache for disks is important 
as
-- 
1.8.0.2

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


[libvirt] [PATCH 1/2] spec: remove redundant %if

2013-01-09 Thread Eric Blake
The daemon-driver-{qemu,lxc} packages are only built if
%{with_driver_modules} is specified, so they do not need to
further test this condition.  Likewise, the daemon package
is only built if %{with_libvirtd} is specified, so it does
not need to further test this condition.

* libvirt.spec.in (daemon-driver-qemu, daemon-driver-lxc):
Unconditionally require libvirt-daemon-driver-network.
(daemon): Unconditionally include lock-driver files.
---
 libvirt.spec.in | 6 --
 1 file changed, 6 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 52da4f4..2eba7c7 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -824,9 +824,7 @@ Summary: Qemu driver plugin for the libvirtd daemon
 Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # There really is a hard cross-driver dependency here
-%if %{with_driver_modules}
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
-%endif

 %description daemon-driver-qemu
 The qemu driver plugin for the libvirtd daemon, providing
@@ -841,9 +839,7 @@ Summary: LXC driver plugin for the libvirtd daemon
 Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # There really is a hard cross-driver dependency here
-%if %{with_driver_modules}
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
-%endif

 %description daemon-driver-lxc
 The LXC driver plugin for the libvirtd daemon, providing
@@ -1743,10 +1739,8 @@ rm -f 
$RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/libvirtd.conf
 %dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/
 %endif

-%if %{with_libvirtd}
 %dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver
 %attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so
-%endif

 %if %{with_qemu}
 %{_datadir}/augeas/lenses/libvirtd_qemu.aug
-- 
1.8.0.2

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


[libvirt] [PATCH 0/2] spec file cleanups

2013-01-09 Thread Eric Blake
While working on recent tarball issues, I noticed the spec file
was hard to read.  In developing the second patch for my own
sanity, I found some cleanups that I separated out to the first
patch.  I don't know if you want to apply the second patch, and
even if you do, it might be nice if 'cppi' could be enhanced to
enforce indentation rules in spec files the same way it does
for preprocessor rules in .c files (maybe even by using a
judicious tr to turn %if into #if of a temporary file, so that
existing cppi could do it); without enforcement, I'm afraid the
second patch will be quickly fall back into hard-to-read status.

Eric Blake (2):
  spec: remove redundant %if
  spec: indent %if to make it easier to see conditions

 libvirt.spec.in | 864 
 1 file changed, 428 insertions(+), 436 deletions(-)

-- 
1.8.0.2

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


[libvirt] [PATCH 2/2] spec: indent %if to make it easier to see conditions

2013-01-09 Thread Eric Blake
* libvirt.spec.in: Add some indentation to make it easier to follow
various conditionals.
---
 libvirt.spec.in | 858 
 1 file changed, 428 insertions(+), 430 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2eba7c7..a567870 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -22,16 +22,16 @@

 # RHEL-5 builds are client-only for s390, ppc
 %if 0%{?rhel} == 5
-%ifnarch %{ix86} x86_64 ia64
-%define client_only1
-%endif
+ %ifnarch %{ix86} x86_64 ia64
+  %define client_only1
+ %endif
 %endif

 # Disable all server side drivers if client only build requested
 %if %{client_only}
-%define server_drivers 0
+ %define server_drivers 0
 %else
-%define server_drivers 1
+ %define server_drivers 1
 %endif

 # Always build with dlopen'd modules
@@ -54,15 +54,15 @@
 %define with_qemu_tcg  %{with_qemu}
 # Change if we ever provide qemu-kvm binaries on non-x86 hosts
 %if 0%{?fedora} = 18
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x
+ %define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x
 %else
-%define qemu_kvm_arches%{ix86} x86_64
+ %define qemu_kvm_arches%{ix86} x86_64
 %endif

 %ifarch %{qemu_kvm_arches}
-%define with_qemu_kvm  %{with_qemu}
+ %define with_qemu_kvm  %{with_qemu}
 %else
-%define with_qemu_kvm  0
+ %define with_qemu_kvm  0
 %endif

 # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
@@ -84,14 +84,14 @@
 %define with_storage_disk 0%{!?_without_storage_disk:%{server_drivers}}
 %define with_storage_mpath0%{!?_without_storage_mpath:%{server_drivers}}
 %if 0%{?fedora} = 16
-%define with_storage_rbd  0%{!?_without_storage_rbd:%{server_drivers}}
+ %define with_storage_rbd  0%{!?_without_storage_rbd:%{server_drivers}}
 %else
-%define with_storage_rbd  0
+ %define with_storage_rbd  0
 %endif
 %if 0%{?fedora} = 17
-%define with_storage_sheepdog 0%{!?_without_storage_sheepdog:%{server_drivers}}
+ %define with_storage_sheepdog 
0%{!?_without_storage_sheepdog:%{server_drivers}}
 %else
-%define with_storage_sheepdog 0
+ %define with_storage_sheepdog 0
 %endif
 %define with_numactl  0%{!?_without_numactl:%{server_drivers}}
 %define with_selinux  0%{!?_without_selinux:%{server_drivers}}
@@ -126,215 +126,215 @@

 # Xen is available only on i386 x86_64 ia64
 %ifnarch %{ix86} x86_64 ia64
-%define with_xen 0
-%define with_libxl 0
+ %define with_xen 0
+ %define with_libxl 0
 %endif

 # Numactl is not available on s390[x] and ARM
 %ifarch s390 s390x %{arm}
-%define with_numactl 0
+ %define with_numactl 0
 %endif

 # RHEL doesn't ship OpenVZ, VBox, UML, PowerHypervisor,
 # VMWare, libxenserver (xenapi), libxenlight (Xen 4.1 and newer),
 # or HyperV.
 %if 0%{?rhel}
-%define with_openvz 0
-%define with_vbox 0
-%define with_uml 0
-%define with_phyp 0
-%define with_vmware 0
-%define with_xenapi 0
-%define with_libxl 0
-%define with_hyperv 0
-%define with_parallels 0
+ %define with_openvz 0
+ %define with_vbox 0
+ %define with_uml 0
+ %define with_phyp 0
+ %define with_vmware 0
+ %define with_xenapi 0
+ %define with_libxl 0
+ %define with_hyperv 0
+ %define with_parallels 0
 %endif

 # Fedora 17 / RHEL-7 are first where we use systemd. Although earlier
 # Fedora has systemd, libvirt still used sysvinit there.
 %if 0%{?fedora} = 17 || 0%{?rhel} = 7
-%define with_systemd 1
+ %define with_systemd 1
 %endif

 # Fedora 18 / RHEL-7 are first where firewalld support is enabled
 %if 0%{?fedora} = 17 || 0%{?rhel} = 7
-%define with_firewalld 1
+ %define with_firewalld 1
 %endif

 # RHEL-5 has restricted QEMU to x86_64 only and is too old for LXC
 %if 0%{?rhel} == 5
-%define with_qemu_tcg 0
-%ifnarch x86_64
-%define with_qemu 0
-%define with_qemu_kvm 0
-%endif
-%define with_lxc 0
+ %define with_qemu_tcg 0
+ %ifnarch x86_64
+  %define with_qemu 0
+  %define with_qemu_kvm 0
+ %endif
+ %define with_lxc 0
 %endif

 # RHEL-6 has restricted QEMU to x86_64 only, stopped including Xen
 # on all archs. Other archs all have LXC available though
 %if 0%{?rhel} = 6
-%define with_qemu_tcg 0
-%ifnarch x86_64
-%define with_qemu 0
-%define with_qemu_kvm 0
-%endif
-%define with_xen 0
+ %define with_qemu_tcg 0
+ %ifnarch x86_64
+  %define with_qemu 0
+  %define with_qemu_kvm 0
+ %endif
+ %define with_xen 0
 %endif

 # Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc
 %if 0%{?fedora}  0%{?fedora}  16
-%ifarch ppc64
-%define with_qemu 0
-%endif
+ %ifarch ppc64
+  %define with_qemu 0
+ %endif
 %endif

 # Fedora doesn't have new enough Xen for libxl until F18
 %if 0%{?fedora}  0%{?fedora}  18
-%define with_libxl 0
+ %define with_libxl 0
 %endif

 # PolicyKit was introduced in Fedora 8 / RHEL-6 or newer
 %if 0%{?fedora} = 8 || 0%{?rhel} = 6
-%define with_polkit0%{!?_without_polkit:1}
+ %define with_polkit0%{!?_without_polkit:1}
 %endif

 # libcapng is used to manage capabilities in Fedora 12 / RHEL-6 or newer
 %if 0%{?fedora} = 12 || 0%{?rhel} = 

Re: [libvirt] [PATCH] build: move file deleting action from %files list to %install

2013-01-09 Thread Eric Blake
On 01/09/2013 05:18 AM, Yufang Zhang wrote:
 When building libvirt rpms on rhel5, I got the following error:
 
 File must begin with /: rm
 File must begin with /: -f
 File must begin with /: $RPM_BUILD_ROOT/etc/sysctl.d/libvirtd
 Installed (but unpackaged) file(s) found:
/etc/sysctl.d/libvirtd
 
 It is triggerd by the %files list of libvirt daemon:
 
 %if 0%{?fedora} = 14 || 0%{?rhel} = 6
 %config(noreplace) %{_prefix}/lib/sysctl.d/libvirtd.conf
 %else
 rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/libvirtd.conf
 %endif
 
 After checking document of rpm spec file, I think it would be better
 to move the file deleting line from %files list to %install script.

Correct intention, but not quite the right patch.

  
 +%if 0%{?fedora}  14 || 0%{?rhel} == 5

Oops; this fails on RHEL 6 (since there, %{?fedora} is 0 so the left
half is always true); and on RHEL 4 (if anyone seriously tries to port
that far back).  The correct inversion of:

%if 0%{?fedora} = 14 || 0%{?rhel} = 6

would be:

%if 0%{?fedoa}  14  0%{?rhel}  6

ACK with that change, and will push shortly.

-- 
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 1/2] add qemu pci-express device support

2013-01-09 Thread li guang
在 2013-01-09三的 10:32 +,Daniel P. Berrange写道:
 On Wed, Jan 09, 2013 at 10:35:32AM +0800, liguang wrote:
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   src/conf/device_conf.c   |8 +++-
   src/conf/device_conf.h   |1 +
   src/qemu/qemu_capabilities.c |   11 +++
   src/qemu/qemu_capabilities.h |1 +
   src/qemu/qemu_command.c  |4 +++-
   5 files changed, 23 insertions(+), 2 deletions(-)
  
  diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
  index 7b97f45..3fca853 100644
  --- a/src/conf/device_conf.c
  +++ b/src/conf/device_conf.c
  @@ -51,7 +51,7 @@ int
   virDevicePCIAddressParseXML(xmlNodePtr node,
   virDevicePCIAddressPtr addr)
   {
  -char *domain, *slot, *bus, *function, *multi;
  +char *domain, *slot, *bus, *function, *multi, *expr;
   int ret = -1;
   
   memset(addr, 0, sizeof(*addr));
  @@ -61,6 +61,7 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
   slot = virXMLPropString(node, slot);
   function = virXMLPropString(node, function);
   multi= virXMLPropString(node, multifunction);
  +expr = virXMLPropString(node, express);
 
 NACK, this is a gross hack.
 
 The q35 machine type provides multiple PCI buses, and it is
 already possible to express connection to the alternative
 buses using the 'bus' parameter. We don't need a new 'express'
 parameter. We need to make sure that the XML includes a
 controller element for each PCI bus provided by a machine
 type
 
 Daniel

OK, will drop this unnecessary patch.

-- 
regards!
li guang


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

Re: [libvirt] [PATCH] qemu_agent: Remove agent reference only when disposing it

2013-01-09 Thread Eric Blake
On 01/09/2013 06:30 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=892079
 
 With current code, if user calls virDomainPMSuspendForDuration()
 followed by virDomainDestroy(), the former API checks for qemu agent
 presence, which will evaluate as true (if agent is configured). While
 talking to qemu agent, the qemu driver is unlocked, so the latter API
 starts executing.  However, if machine dies meanwhile, libvirtd gets
 EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The
 handler clears reference to qemu agent while the destroy API already
 holding a reference to it. This leads to NULL dereferencing later in
 the code. Therefore, the agent pointer should be set to NULL only if
 we are the exclusive owner of it.
 ---
 
 There's a reproducer in the BZ. It doesn't have to be a windows guest,
 I was able to reproduce with F17 guest as well.
 
  src/qemu/qemu_process.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

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

[libvirt] [PATCH v3 3/3] schemas: add pci-bridge type for controller

2013-01-09 Thread liguang
Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
 docs/schemas/domaincommon.rng |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0529d62..f4c22b6 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1324,6 +1324,7 @@
 valuesata/value
 valueccid/value
 valueusb/value
+valuepci-bridge/value
   /choice
 /attribute
   /optional
-- 
1.7.2.5

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


Re: [libvirt] [PATCH] Move comment after enum members

2013-01-09 Thread Eric Blake
On 01/09/2013 07:20 AM, Claudio Bley wrote:
 The api builder always associates comments to the last member it read,
 not to the current member even if there was a comment for the previous
 member and a comma was already seen.
 
 This has the effect that the comment for the previous member gets
 overwritten and the current member has no comment at all.
 
 Signed-off-by: Claudio Bley cb...@av-test.de
 ---
  include/libvirt/libvirt.h.in |   18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 09c89c5..9110fcf 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -127,12 +127,12 @@ typedef enum {
  power management */
  
  #ifdef VIR_ENUM_SENTINELS
 -/*
 - * NB: this enum value will increase over time as new events are
 - * added to the libvirt API. It reflects the last state supported
 - * by this version of the libvirt API.
 - */
   VIR_DOMAIN_LAST
 + /*

You changed indentation from 4 to 5; please fix that.  Oh, I see why -
VIR_DOMAIN_LAST is incorrectly indented to begin with.

 +  * NB: this enum value will increase over time as new events are
 +  * added to the libvirt API. It reflects the last state supported
 +  * by this version of the libvirt API.
 +  */
  #endif
  } virDomainState;

Hmm, I wonder if we should instead fix the doc-generation parser.  I'm
used to the style:

ONE, /* short comment for one */
/* longer comment for two */
TWO,
THREE, /* and short for three again */

But I guess I could live with this patch, if the style is forced on us
by the doc generator.  Anyone else with an opinion?

-- 
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 v3 0/3] add pci-bridge support

2013-01-09 Thread liguang
Now, it's impossible to arrange devices into multi-pci-bus,
for example:
   sound model='ac97'
  address type='pci' domain='0x' bus='0x00' slot='0x04' 
function='0x0'/
   /sound
   video
  model type='cirrus' vram='9216' heads='1'/
  address type='pci' domain='0x' bus='0x1' slot='0x02' function='0x0'/
   /video
libvirt will complain about bus != 0,
fortunately, qemu supports pci-to-pci bridge,
if we want to use multi-pci-bus, we can define
2 pci bridge controllers, then attach 1 to the other
as a subordinate pci-bus, so, 2 pci-buses appear.
for example:
   controller type='pci-bridge' index='0'/
   controller type='pci-bridge' index='1'
  address type='pci' domain='0x' bus='0x00' slot='0x05' 
function='0x0'/
   /controller
   sound model='ac97'
  address type='pci' domain='0x' bus='0x01' slot='0x02' 
function='0x0'/
   /sound
   video
  model type='cirrus' vram='9216' heads='1'/
  address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
   /video


 src/conf/domain_conf.c|5 -
 src/conf/domain_conf.h|1 +
 docs/schemas/domaincommon.rng |1 +
 src/qemu/qemu_capabilities.c  |2 +
 src/qemu/qemu_capabilities.h  |1 +
 src/qemu/qemu_command.c   |   70 -
 6 files changed, 57 insertions(+), 23 deletions(-)

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


[libvirt] [PATCH v3 1/3] add pci-bridge controller type

2013-01-09 Thread liguang
add a new controller type, then one can
define a pci-bridge controller like this:
controller type='pci-bridge' index='0'/
controller type='pci-bridge' index='1'
  address type='pci' domain='0x' bus='0x00' slot='0x05' 
function='0x0'/
/controller
actually, it works as a pci-bus, so as to support
multi-pci-bus via pci-to-pci bridge

Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
 src/conf/domain_conf.c |5 -
 src/conf/domain_conf.h |1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a7646e..8ebe77d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -264,7 +264,8 @@ VIR_ENUM_IMPL(virDomainController, 
VIR_DOMAIN_CONTROLLER_TYPE_LAST,
   sata,
   virtio-serial,
   ccid,
-  usb)
+  usb,
+  pci-bridge)
 
 VIR_ENUM_IMPL(virDomainControllerModelSCSI, 
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
   auto,
@@ -4479,6 +4480,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
 goto error;
 
 switch (def-type) {
+case VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE:
+break;
 case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: {
 char *ports = virXMLPropString(node, ports);
 if (ports) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5062e07..56e5a40 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -652,6 +652,7 @@ enum virDomainControllerType {
 VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
 VIR_DOMAIN_CONTROLLER_TYPE_CCID,
 VIR_DOMAIN_CONTROLLER_TYPE_USB,
+VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE,
 
 VIR_DOMAIN_CONTROLLER_TYPE_LAST
 };
-- 
1.7.2.5

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


[libvirt] [PATCH v3 2/3] build command line for pci-bridge device of qemu

2013-01-09 Thread liguang
format command line of qemu to add pci-bridge
like this: -device pci-bridge.
and also add a qemu capability to check if
qemu support pci-bridge device

Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
 src/qemu/qemu_capabilities.c |2 +
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_command.c  |   70 -
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 104a3f8..90c08b9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -200,6 +200,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   cirrus-vga,
   vmware-svga,
   device-video-primary,
+  pci-bridge,
 );
 
 struct _qemuCaps {
@@ -1339,6 +1340,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = {
 { VGA, QEMU_CAPS_DEVICE_VGA },
 { cirrus-vga, QEMU_CAPS_DEVICE_CIRRUS_VGA },
 { vmware-svga, QEMU_CAPS_DEVICE_VMWARE_SVGA },
+{ pci-bridge, QEMU_CAPS_DEVICE_PCI_BRIDGE },
 };
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bf4eef8..64fc73d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -162,6 +162,7 @@ enum qemuCapsFlags {
 QEMU_CAPS_DEVICE_VMWARE_SVGA = 122, /* -device vmware-svga */
 QEMU_CAPS_DEVICE_VIDEO_PRIMARY = 123, /* safe to use -device XXX
for primary video device */
+QEMU_CAPS_DEVICE_PCI_BRIDGE  = 124, /* -device pci-bridge */
 
 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 04a9512..48b4f46 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -966,13 +966,6 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr 
dev)
 {
 char *addr;
 
-if (dev-addr.pci.domain != 0 ||
-dev-addr.pci.bus != 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(Only PCI domain 0 and bus 0 are available));
-return NULL;
-}
-
 if (virAsprintf(addr, %d:%d:%d.%d,
 dev-addr.pci.domain,
 dev-addr.pci.bus,
@@ -984,8 +977,24 @@ static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr 
dev)
 return addr;
 }
 
+static int qemuPciBridgeSupport(virDomainDefPtr def)
+{
+int i, c = 0;
+
+for (i = 0; i  def-ncontrollers; i++) {
+virDomainControllerDefPtr controller = def-controllers[i];
+
+if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE)
+c++;
+}
 
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
+if (c  1)
+return 0;
+else
+return -1;
+}
+
+static int qemuCollectPCIAddress(virDomainDefPtr def,
  virDomainDeviceDefPtr device,
  virDomainDeviceInfoPtr info,
  void *opaque)
@@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 return 0;
 }
 
+if (info-addr.pci.domain != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Only PCI device addresses with 
+ domain=0 are supported));
+return -1;
+}
+
+if (info-addr.pci.bus != 0  qemuPciBridgeSupport(def)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Only PCI device addresses with bus=0 are
+  supported without pci-bridge support));
+return -1;
+}
+
 addr = qemuPCIAddressAsString(info);
 if (!addr)
 goto cleanup;
@@ -1012,7 +1035,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 if (info-addr.pci.function != 0) {
 virReportError(VIR_ERR_XML_ERROR,
_(Attempted double use of PCI Address '%s' 
- (may need \multifunction='on'\ for device on 
function 0)),
+ (may need \multifunction='on'\ for 
+ device on function 0)),
addr);
 } else {
 virReportError(VIR_ERR_XML_ERROR,
@@ -1047,7 +1071,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-VIR_DEBUG(Remembering PCI addr %s (multifunction=off for function 
0), addr);
+VIR_DEBUG(Remembering PCI addr %s (multifunction=off
+   for function 0), addr);
 if (virHashAddEntry(addrs-used, addr, addr))
 goto cleanup;
 addr = NULL;
@@ -1763,16 +1788,6 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
   qemuCapsPtr caps)
 {
 if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-if (info-addr.pci.domain != 0) {
-

[libvirt] API to upgrade read-only connection

2013-01-09 Thread Zeeshan Ali (Khattak)
Hi,
  Once again, I'll be lazy and just copypaste an IRC conversation but
please don't hesitate to ask if something needs clarification:

zeenix am i missing something or there is no way to 'upgrade' a
read-only connection to a normal one?
eblake_out zeenix: looks like you have to create a new connection if
you want new privileges
eblake_out although you may want to float it by the list to see if a
new API for upgrading an existing connection makes sense
eblake_out especially in light of danpb's work-in-progress on adding
fine-grained ACLs
zeenix ah ok
zeenix eblake_out: we'd like to connect to system libvirt as well by
default in boxes
zeenix but would be nice to avoid the polkit dialog until we really
need full-access

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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


Re: [libvirt] [PATCH 11/14] xen: Resource resource leak with 'cpuset'

2013-01-09 Thread Laine Stump
On 01/09/2013 01:50 PM, John Ferlan wrote:
 On 01/09/2013 11:55 AM, Laine Stump wrote:
 (you duplicated resource in the subject line)

 Missed that one... Will fix.

 On 01/09/2013 09:54 AM, John Ferlan wrote:
 Make cpuset local to the while loop and free it once done with it each
 time through the loop.
 ---
  src/xen/xend_internal.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index 84a25e8..c6b800b 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -1113,7 +1113,6 @@ sexpr_to_xend_topology(const struct sexpr *root,
  {
  const char *nodeToCpu;
  const char *cur;
 -virBitmapPtr cpuset = NULL;
  int *cpuNums = NULL;
  int cell, cpu, nb_cpus;
  int n = 0;
 @@ -1131,6 +1130,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
  
  cur = nodeToCpu;
  while (*cur != 0) {
 +virBitmapPtr cpuset = NULL;
  /*
   * Find the next NUMA cell described in the xend output
   */
 @@ -1152,8 +1152,10 @@ sexpr_to_xend_topology(const struct sexpr *root,
  goto memory_error;
  } else {
  nb_cpus = virBitmapParse(cur, 'n', cpuset, numCpus);
 -if (nb_cpus  0)
 +if (nb_cpus  0) {
 +virBitmapFree(cpuset);
 This virBitmapFree() isn't necessary - virBitmapParse is guaranteed to
 have nothing allocated (and will set cpuset = NULL) if it fails.

 According to Coverity's analysis this may not be true since it's
 possible to hit the ret-- line (more than once) in virBitmapParse()
 while hitting either ret++ line less times returning a negative value
 on the success path. The example Coverity had shows 6 passes through
 the loop, 4 negatives, 1 positive, and 1 nothing.

 Whether realistically this could be true, I am not sure.

 How Coverity determined what the value of 'cpuSet' is a mystery as the
 output I have doesn't show what's being used for parsing, just that we
 go through the loop 6 times. Perhaps something like ^1,^2,^3,4,^5,^6
 where 1,2,3,4,5,6 pass the virBitmapIsSet() call changing the 'ret'
 value to -3.

I don't think that is possible. In order for virBitmapIsSet() to return
true for a particular bit, that bit must be set, and in order for that
bit to be set, it must have been set in a previous iteration of this
same loop (remember that the bitmap is initialized to all empty at the
top of the function), which means that ret++ must have been executed. So
ret-- can't happen without a previous corresponding ret++, therefore the
value of ret can't be  0.

If it was possible to have a return  0 on success, that would be a bug
in the function that would need to be fixed.


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


Re: [libvirt] [Qemu-devel] [Qemu PATCH v2] add a boot option to do strict boot

2013-01-09 Thread Laine Stump
On 01/09/2013 01:02 PM, Daniel P. Berrange wrote:
 On Wed, Jan 09, 2013 at 12:36:52PM -0500, Laine Stump wrote:
 On 01/09/2013 10:52 AM, Amos Kong wrote:
 On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote:
 On 01/09/2013 01:39 AM, Amos Kong wrote:
 Current seabios will try to boot from selected devices first,
 if they are all failed, seabios will also try to boot from
 un-selected devices.

 We need to make it configurable. I already posted a seabios
 patch to add a new device type to halt booting. Qemu can add
 HALT at the end of bootindex string, then seabios will halt
 booting after trying to boot from selected devices.

 This option only effects when boot priority is changed by
 bootindex options, the old style(-boot order=..) will still
 try to boot from un-selected devices.

 v2: add HALT entry in get_boot_devices_list()
 define boot_strict to bool

 Signed-off-by: Amos Kong ak...@redhat.com
 ---
 Libvirt will need to expose an attribute that lets the user control
 whether to use this new option; how do we probe via QMP whether the new
 -boot strict=on command-line option is available?
 Old style to adjust boot priority by order parameter:
 -boot order=n,strict=on (BAD, unselected devices will be tried)

 New style to adjust boot priority by bootindex:
 -device virtio-net-pci,...,bootindex=1 -boot strict=on (OK)

 We only want strict option to support new style.

 (those two styles are implemented in two different way insider
 seabios, the latest simple patch only changed the bootindex way)
 Just a note about this: as far as I can tell virt-manager currently only
 uses the old style of specifying boot order; it needs to be enhanced
 to recognize the presence of bootindex=n ordering, and behave
 accordingly. Looking from the outside, that looks to be not completely
 trivial, as the old style of ordering only allows specifying hard disk
 as a single line in the priority order, but the new style has each disk
 specified separately. Not only that, but *which* of the disks is tried
 first under the old order may change depending on whether or not a
 bootmenu is requested (in one case it picks unit=0 on the controller, in
 the other case, it orders the disks alphabetically by target dev name)
 Hmm, well libvirt should be using bootindex=n on the QEMU command line
 regardless of what applications put in the XML. ie if the application
 uses the old style XML, libvirt should translate that into bootindex=n
 for them.

Good point. WHerever it's changed though, it will be a bit tricky to
preserve current behavior, due to the idiosyncrasies I noted above.

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


Re: [libvirt] [PATCH 01/14] phyp: Resolve some file descriptor leaks

2013-01-09 Thread Eric Blake
On 01/09/2013 07:54 AM, John Ferlan wrote:
 The phypUUIDTable_Push and phypUUIDTable_Pull leaked their fd's on normal
 return.  I also noted that the Read function had a cut-n-paste error from
 the write function on a couple of VIR_WARN's
 
 The openSSHSession leaked the sock on the failure path.  Additionally that
 turns into the internal_socket in the phypOpen code.  That was neither saved
 nor closed on any path. So I used the connnection_data-sock field to save
 the socket for eventual close. Of interest here is that phypExec used the
 connection_data-sock field even though it had never been initialized.
 ---
  src/phyp/phyp_driver.c | 19 ++-
  1 file changed, 14 insertions(+), 5 deletions(-)
 
 diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
 index f89871f..b74cab8 100644
 --- a/src/phyp/phyp_driver.c
 +++ b/src/phyp/phyp_driver.c
 @@ -577,6 +577,7 @@ phypUUIDTable_Push(virConnectPtr conn)
  libssh2_channel_free(channel);
  channel = NULL;
  }
 +VIR_FORCE_FCLOSE(fd);

Who named their FILE* 'fd' anyways?  But not your fault that you are
trying to clean up some notoriously hairy code.

  virBufferFreeAndReset(username);
  return 0;

Hmm, the code feels rather repetitive:

if (channel) {
libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel);
libssh2_channel_wait_closed(channel);
libssh2_channel_free(channel);
channel = NULL;
}
VIR_FORCE_FCLOSE(fd);
virBufferFreeAndReset(username);
return 0;

err:
if (channel) {
libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel);
libssh2_channel_wait_closed(channel);
libssh2_channel_free(channel);
channel = NULL;
}
VIR_FORCE_FCLOSE(fd);
return -1;
}

Also, the virBufferFreeAndReset(username) on the success path is
useless (the only way to get there is if username was already freed
earlier), and the rest of the code is identical except for the return
value.  Oh, and the entire 'username' variable is pointless - it is
either empty or precisely conn-uri-user; no need to malloc a buffer to
copy a string when we can just use the string directly.

How about we do this instead:

diff --git i/src/phyp/phyp_driver.c w/src/phyp/phyp_driver.c
index f89871f..87a94c0 100644
--- i/src/phyp/phyp_driver.c
+++ w/src/phyp/phyp_driver.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
  * Copyright IBM Corp. 2009
  *
  * phyp_driver.c: ssh layer to access Power Hypervisors
@@ -493,42 +493,30 @@ phypUUIDTable_Push(virConnectPtr conn)
 ConnectionData *connection_data = conn-networkPrivateData;
 LIBSSH2_SESSION *session = connection_data-session;
 LIBSSH2_CHANNEL *channel = NULL;
-virBuffer username = VIR_BUFFER_INITIALIZER;
 struct stat local_fileinfo;
 char buffer[1024];
 int rc = 0;
-FILE *fd = NULL;
+FILE *f = NULL;
 size_t nread, sent;
 char *ptr;
 char local_file[] = ./uuid_table;
 char *remote_file = NULL;
+int ret = -1;

-if (conn-uri-user != NULL) {
-virBufferAdd(username, conn-uri-user, -1);
-
-if (virBufferError(username)) {
-virBufferFreeAndReset(username);
-virReportOOMError();
-goto err;
-}
-}
-
-if (virAsprintf
-(remote_file, /home/%s/libvirt_uuid_table,
- virBufferContentAndReset(username))
- 0) {
+if (virAsprintf(remote_file, /home/%s/libvirt_uuid_table,
+NULLSTR(conn-uri-user))  0) {
 virReportOOMError();
-goto err;
+goto cleanup;
 }

 if (stat(local_file, local_fileinfo) == -1) {
 VIR_WARN(Unable to stat local file.);
-goto err;
+goto cleanup;
 }

-if (!(fd = fopen(local_file, rb))) {
+if (!(f = fopen(local_file, rb))) {
 VIR_WARN(Unable to open local file.);
-goto err;
+goto cleanup;
 }

 do {
@@ -539,18 +527,18 @@ phypUUIDTable_Push(virConnectPtr conn)

 if ((!channel)  (libssh2_session_last_errno(session) !=
LIBSSH2_ERROR_EAGAIN))
-goto err;
+goto cleanup;
 } while (!channel);

 do {
-nread = fread(buffer, 1, sizeof(buffer), fd);
+nread = fread(buffer, 1, sizeof(buffer), f);
 if (nread = 0) {
-if (feof(fd)) {
+if (feof(f)) {
 /* end of file */
 break;
 } else {
 VIR_ERROR(_(Failed to read from %s), local_file);
-goto err;
+goto cleanup;
 }
 }
 ptr = buffer;
@@ -570,17 +558,9 @@ phypUUIDTable_Push(virConnectPtr conn)
 } while (rc  0  sent  nread);
 } while (1);

-if (channel) {
-libssh2_channel_send_eof(channel);
-libssh2_channel_wait_eof(channel);
-

Re: [libvirt] [PATCH 0/2] add qemu machine type q35 support

2013-01-09 Thread Laine Stump
On 01/08/2013 10:15 PM, Doug Goldstein wrote:
 On Tue, Jan 8, 2013 at 8:35 PM, liguang lig.f...@cn.fujitsu.com wrote:
 qemu-1.3 added machine type q35,
 changelog said:
 Added Intel Q35 chipset as a new machine type,
 '--machine q35'. Adds PCIe support. Requires an updated SeaBIOS (bios.bin),
 and '-acpitable file=/seabios-path/q35-acpi-dsdt.aml' to run.
 so add q35 support for libvirt.

  src/conf/device_conf.c   |8 +++-
  src/conf/device_conf.h   |1 +
  src/conf/domain_conf.c   |1 +
  src/conf/domain_conf.h   |1 +
  src/qemu/qemu_capabilities.c |   11 +++
  src/qemu/qemu_capabilities.h |1 +
  src/qemu/qemu_command.c  |8 +++-
  7 files changed, 29 insertions(+), 2 deletions(-)

 I'd personally NACK this series for the time being. Per the qemu
 maintainers, q35 isn't really fully ready until 1.4. They're actively
 in the process of hashing out the machine type which will be exposed
 on the command line and via QMP so I think we really need to wait
 until that lands in upstream's repo before we add code for it in
 libvirt.

In addition there is the issue that libvirt currently makes assumptions
about the bus topology of guests, and assigns pci addresses to guest
devices accordingly. Beyond the basic layout of buses present on the
machine, certain addresses are reserved/used for certain default devices
(e.g. the first video device). The q35 machine type will have a
different topology, so the default addresses of these basic devices may
(will? haven't looked at the details yet) change, and different
addresses will be available for assigning to additional guest pci
devices. (This assumption was actually already a problem, because
libvirt currently incorrectly assumes the default addresses and bus
layout even for non-x86 machinetypes.)

To properly solve this problem, libvirt will need a method of learning
the topology and default device address assignment for each machine type
during capabilities discovery, and honor that information (rather than
just making decisions based on assumptions) during guest device assignment.

I'm already working with someone from qemu on a design and patches to
implement this functionality, so to avoid duplication of effort, you
should wait to see what that looks like before you do any more work in
this area. (A colleague has also been working on PCI bridge support,
which has the potential to conflict with the PCI-e/q35 work if not done
in a coordinated fashion, so you may want to take a wait-and-see
approach there as well.)

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


Re: [libvirt] [PATCH 11/14] xen: Resource resource leak with 'cpuset'

2013-01-09 Thread Eric Blake
On 01/09/2013 07:07 PM, Laine Stump wrote:

 According to Coverity's analysis this may not be true since it's
 possible to hit the ret-- line (more than once) in virBitmapParse()
 while hitting either ret++ line less times returning a negative value
 on the success path. The example Coverity had shows 6 passes through
 the loop, 4 negatives, 1 positive, and 1 nothing.

 Whether realistically this could be true, I am not sure.

 How Coverity determined what the value of 'cpuSet' is a mystery as the
 output I have doesn't show what's being used for parsing, just that we
 go through the loop 6 times. Perhaps something like ^1,^2,^3,4,^5,^6
 where 1,2,3,4,5,6 pass the virBitmapIsSet() call changing the 'ret'
 value to -3.

Except that we would have rejected '^1' outright up front.

 
 I don't think that is possible. In order for virBitmapIsSet() to return
 true for a particular bit, that bit must be set, and in order for that
 bit to be set, it must have been set in a previous iteration of this
 same loop (remember that the bitmap is initialized to all empty at the
 top of the function), which means that ret++ must have been executed. So
 ret-- can't happen without a previous corresponding ret++, therefore the
 value of ret can't be  0.

Maybe a well-placed:

sa_assert(ret = 0);

will help Coverity learn a bit more of the logic flow, which proves that
we cannot reach the success path with too many ret-- lines.

 
 If it was possible to have a return  0 on success, that would be a bug
 in the function that would need to be fixed.

-- 
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 0/2] add qemu machine type q35 support

2013-01-09 Thread li guang
在 2013-01-09三的 21:24 -0500,Laine Stump写道:
 On 01/08/2013 10:15 PM, Doug Goldstein wrote:
  On Tue, Jan 8, 2013 at 8:35 PM, liguang lig.f...@cn.fujitsu.com wrote:
  qemu-1.3 added machine type q35,
  changelog said:
  Added Intel Q35 chipset as a new machine type,
  '--machine q35'. Adds PCIe support. Requires an updated SeaBIOS (bios.bin),
  and '-acpitable file=/seabios-path/q35-acpi-dsdt.aml' to run.
  so add q35 support for libvirt.
 
   src/conf/device_conf.c   |8 +++-
   src/conf/device_conf.h   |1 +
   src/conf/domain_conf.c   |1 +
   src/conf/domain_conf.h   |1 +
   src/qemu/qemu_capabilities.c |   11 +++
   src/qemu/qemu_capabilities.h |1 +
   src/qemu/qemu_command.c  |8 +++-
   7 files changed, 29 insertions(+), 2 deletions(-)
 
  I'd personally NACK this series for the time being. Per the qemu
  maintainers, q35 isn't really fully ready until 1.4. They're actively
  in the process of hashing out the machine type which will be exposed
  on the command line and via QMP so I think we really need to wait
  until that lands in upstream's repo before we add code for it in
  libvirt.
 
 In addition there is the issue that libvirt currently makes assumptions
 about the bus topology of guests, and assigns pci addresses to guest
 devices accordingly. Beyond the basic layout of buses present on the
 machine, certain addresses are reserved/used for certain default devices
 (e.g. the first video device). The q35 machine type will have a
 different topology, so the default addresses of these basic devices may
 (will? haven't looked at the details yet) change, and different
 addresses will be available for assigning to additional guest pci
 devices. (This assumption was actually already a problem, because
 libvirt currently incorrectly assumes the default addresses and bus
 layout even for non-x86 machinetypes.)
 
 To properly solve this problem, libvirt will need a method of learning
 the topology and default device address assignment for each machine type
 during capabilities discovery, and honor that information (rather than
 just making decisions based on assumptions) during guest device assignment.
 
 I'm already working with someone from qemu on a design and patches to
 implement this functionality, so to avoid duplication of effort, you
 should wait to see what that looks like before you do any more work in
 this area. (A colleague has also been working on PCI bridge support,
 which has the potential to conflict with the PCI-e/q35 work if not done
 in a coordinated fashion, so you may want to take a wait-and-see
 approach there as well.)
 

glad to hear that, 
I've committed some patches to support pci-bridge,
https://www.redhat.com/archives/libvir-list/2013-January/msg00577.html

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

-- 
regards!
li guang


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

[libvirt] Attaching an Internal Address

2013-01-09 Thread Varun Bhatnagar
Hi,

I am trying to attach an internal address to my domain. So for that I am 
defining the following syntax in my xml file:




but it is giving the following error:



Tried giving interface type as network but that is adding Host-only type 
interface.
Can anyone please suggest what can be the possible solution. I think it is 
some problem with the source network. Do I have to define any internal 
network before in any interface?
If yes then how?

Any help will be appreciated.
Thanks in advance.

Regards,
Varun
=-=-=
Notice: The information contained in this e-mail
message and/or attachments to it may contain 
confidential or privileged information. If you are 
not the intended recipient, any dissemination, use, 
review, distribution, printing or copying of the 
information contained in this e-mail message 
and/or attachments to it are strictly prohibited. If 
you have received this communication in error, 
please notify us by reply e-mail or telephone and 
immediately and permanently delete the message 
and any attachments. Thank you


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

[libvirt] [PATCH] pass stub driver name instead of pciFindStubDriver

2013-01-09 Thread Chunyan Liu
Pass stub driver name directly to pciDettachDevice and pciReAttachDevice to fit
for different libvirt drivers. For example, qemu driver prefers pci-stub, but
Xen prefers pciback.
 
Signed-off-by: Chunyan Liu cy...@suse.com
---
 src/qemu/qemu_driver.c  |4 +-
 src/qemu/qemu_hostdev.c |6 ++--
 src/util/virpci.c   |   62 +++---
 src/util/virpci.h   |6 +++-
 src/xen/xen_driver.c|4 +-
 5 files changed, 31 insertions(+), 51 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 06b0d28..3427d3f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10022,7 +10022,7 @@ qemuNodeDeviceDettach(virNodeDevicePtr dev)
 in_inactive_list = pciDeviceListFind(driver-inactivePciHostdevs, pci);
 
 if (pciDettachDevice(pci, driver-activePciHostdevs,
- driver-inactivePciHostdevs)  0)
+ driver-inactivePciHostdevs, pci-stub)  0)
 goto out;
 
 ret = 0;
@@ -10067,7 +10067,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
 
 qemuDriverLock(driver);
 if (pciReAttachDevice(pci, driver-activePciHostdevs,
-  driver-inactivePciHostdevs)  0)
+  driver-inactivePciHostdevs, pci-stub)  0)
 goto out;
 
 ret = 0;
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 174d125..896ad9b 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -458,7 +458,7 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
 for (i = 0; i  pciDeviceListCount(pcidevs); i++) {
 pciDevice *dev = pciDeviceListGet(pcidevs, i);
 if (pciDeviceGetManaged(dev) 
-pciDettachDevice(dev, driver-activePciHostdevs, NULL)  0)
+pciDettachDevice(dev, driver-activePciHostdevs, NULL, pci-stub) 
 0)
 goto reattachdevs;
 }
 
@@ -574,7 +574,7 @@ resetvfnetconfig:
 reattachdevs:
 for (i = 0; i  pciDeviceListCount(pcidevs); i++) {
 pciDevice *dev = pciDeviceListGet(pcidevs, i);
-pciReAttachDevice(dev, driver-activePciHostdevs, NULL);
+pciReAttachDevice(dev, driver-activePciHostdevs, NULL, pci-stub);
 }
 
 cleanup:
@@ -830,7 +830,7 @@ void qemuReattachPciDevice(pciDevice *dev, virQEMUDriverPtr 
driver)
 }
 
 if (pciReAttachDevice(dev, driver-activePciHostdevs,
-  driver-inactivePciHostdevs)  0) {
+  driver-inactivePciHostdevs, pci-stub)  0) {
 virErrorPtr err = virGetLastError();
 VIR_ERROR(_(Failed to re-attach PCI device: %s),
   err ? err-message : _(unknown error));
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 4b26452..fb305e2 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -855,57 +855,35 @@ pciDeviceFile(char **buffer, const char *device, const 
char *file)
 return 0;
 }
 
-
-static const char *
-pciFindStubDriver(void)
+static int
+pciProbeStubDriver(const char *driver)
 {
 char *drvpath = NULL;
 int probed = 0;
 
 recheck:
-if (pciDriverDir(drvpath, pci-stub)  0) {
-return NULL;
-}
-
-if (virFileExists(drvpath)) {
-VIR_FREE(drvpath);
-return pci-stub;
-}
-
-if (pciDriverDir(drvpath, pciback)  0) {
-return NULL;
-}
-
-if (virFileExists(drvpath)) {
+if (pciDriverDir(drvpath, driver) == 0  virFileExists(drvpath)) {
+/* driver already loaded, return */
 VIR_FREE(drvpath);
-return pciback;
+return 0;
 }
 
 VIR_FREE(drvpath);
 
 if (!probed) {
-const char *const stubprobe[] = { MODPROBE, pci-stub, NULL };
-const char *const backprobe[] = { MODPROBE, pciback, NULL };
-
+const char *const probecmd[] = { MODPROBE, driver, NULL };
 probed = 1;
-/*
- * Probing for pci-stub will succeed regardless of whether
- * on native or Xen kernels.
- * On Xen though, we want to prefer pciback, so probe
- * for that first, because that will only work on Xen
- */
-if (virRun(backprobe, NULL)  0 
-virRun(stubprobe, NULL)  0) {
+if (virRun(probecmd, NULL)  0 ) {
 char ebuf[1024];
-VIR_WARN(failed to load pci-stub or pciback drivers: %s,
+VIR_WARN(failed to load driver %s: %s, driver,
  virStrerror(errno, ebuf, sizeof(ebuf)));
-return NULL;
+return -1;
 }
 
 goto recheck;
 }
 
-return NULL;
+return -1;
 }
 
 static int
@@ -1149,12 +1127,12 @@ cleanup:
 int
 pciDettachDevice(pciDevice *dev,
  pciDeviceList *activeDevs,
- pciDeviceList *inactiveDevs)
+ pciDeviceList *inactiveDevs,
+ const char *driver)
 {
-const char *driver = pciFindStubDriver();
-if (!driver) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-