Re: [libvirt] [PATCH] virsh: add --start option to the define command
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
--- 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
--- 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
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'
--- 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'
--- 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'
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'
--- 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
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)
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
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
--- 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
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
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
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.
--- 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
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
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)
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
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
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)
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
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
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
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
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'
(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
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
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
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
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
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'
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
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
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
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
* 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
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
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
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
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
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
* 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
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三的 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
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
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
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
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
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
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
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'
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
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
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
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'
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三的 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
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
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, -