[libvirt] [test-API][PATCH] Add volume resize case with delta flag
Only storage backend for RBD (RADOS Block Device), FS and directory
have the resizeVol function, so only testing dir volume here.
Flags 'allocate' and 'shrik' are with bug:
https://bugzilla.redhat.com/show_bug.cgi?id=804516
they are not supported yet, so leave the case for later.
* using volume resize API with flag VIR_STORAGE_VOL_RESIZE_DELTA
* using volume info API to get volume info and check
* add dir volume resize conf
Signed-off-by: Wayne Sun g...@redhat.com
---
cases/storage_dir_vol_resize_delta.conf | 47 +++
repos/storage/vol_resize_delta.py | 75 +++
2 files changed, 122 insertions(+), 0 deletions(-)
create mode 100644 cases/storage_dir_vol_resize_delta.conf
create mode 100644 repos/storage/vol_resize_delta.py
diff --git a/cases/storage_dir_vol_resize_delta.conf
b/cases/storage_dir_vol_resize_delta.conf
new file mode 100644
index 000..58e15bf
--- /dev/null
+++ b/cases/storage_dir_vol_resize_delta.conf
@@ -0,0 +1,47 @@
+storage:create_dir_pool
+poolname
+$defaultpoolname
+
+storage:create_dir_volume
+poolname
+$defaultpoolname
+volname
+$defaultvolumename
+volformat
+$defaultvolumetype
+capacity
+$defaultvolumesize
+
+storage:vol_resize_delta
+poolname
+$defaultpoolname
+volname
+$defaultvolumename
+capacity
+1M
+
+storage:vol_resize_delta
+poolname
+$defaultpoolname
+volname
+$defaultvolumename
+capacity
+2G
+
+storage:vol_resize_delta
+poolname
+$defaultpoolname
+volname
+$defaultvolumename
+capacity
+4096K
+
+storage:delete_dir_volume
+poolname
+$defaultpoolname
+volname
+$defaultvolumename
+
+storage:destroy_pool
+poolname
+$defaultpoolname
diff --git a/repos/storage/vol_resize_delta.py
b/repos/storage/vol_resize_delta.py
new file mode 100644
index 000..a87941e
--- /dev/null
+++ b/repos/storage/vol_resize_delta.py
@@ -0,0 +1,75 @@
+#!/usr/bin/env python
+# volume resize testing with delta flags, libvirt storage
+# driver only support dir now
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+from utils import utils
+
+required_params = ('poolname', 'volname', 'capacity',)
+optional_params = {}
+
+def vol_resize_delta(params):
+test volume resize with delta flags
+
+global logger
+logger = params['logger']
+poolname = params['poolname']
+volname = params['volname']
+capacity = params['capacity']
+
+logger.info(the poolname is %s, volname is %s %
+(poolname, volname))
+
+logger.info(the capacity given is %s % capacity)
+out = utils.get_capacity_suffix_size(capacity)
+capacity_val = out['capacity_byte']
+logger.debug(the capacity to byte is %s % capacity_val)
+
+conn = sharedmod.libvirtobj['conn']
+try:
+poolobj = conn.storagePoolLookupByName(poolname)
+vol = poolobj.storageVolLookupByName(volname)
+
+logger.info(get volume info before resize)
+out = vol.info()
+pre_capacity = out[1]
+pre_allocation = out[2]
+logger.info(volume capacity is %s bytes, allocation is %s bytes %
+(pre_capacity, pre_allocation))
+
+flag = libvirt.VIR_STORAGE_VOL_RESIZE_DELTA
+logger.info(resize %s with capacity %s in pool %s using flag: %s
+% (volname, capacity, poolname, flag))
+
+vol.resize(capacity_val, flag)
+
+logger.info(get volume info after resize)
+out = vol.info()
+post_capacity = out[1]
+post_allocation = out[2]
+logger.info(volume capacity is %s bytes, allocation is %s bytes %
+(post_capacity, post_allocation))
+
+logger.info(check resize effect)
+if post_capacity - pre_capacity == capacity_val:
+logger.info(increased size is expected)
+else:
+logger.error(increase size not equal to set, resize failed)
+return 1
+
+if pre_allocation == post_allocation:
+logger.info(allocation is expected)
+else:
+logger.error(allocation changed, resize failed)
+return 1
+
+logger.info(resize succeed)
+
+except libvirtError, e:
+logger.error(libvirt call failed: + str(e))
+return 1
+
+return 0
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Setup libvirt deveplopment environment
On Mon, Jan 7, 2013 at 7:06 AM, Thomas Lin thomasli...@gmail.com wrote: How to setup libvirt development environment on Ubuntu 12.04? I use virt-manager right now. It already installed virsh with it. I configured libvirt and it all went well. So how to test my modification in source code? When I open ./virsh from libvirt/tools/, it give errors like cannot open socket. Please help, I am a newbie. This helped me in the past: http://aglitke.wordpress.com/2011/02/24/how-to-hack-on-a-local-copy-of-libvirt/ Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] Public API to allow defining new domain using OVA file
On Sat, Jan 05, 2013 at 11:33:51PM -0800, Ata E Husain Bohra wrote: Appends a new API to libvirt public driver that supports defining a new domain using OVA format. API expects following inputs: 1. connection pointer. 2. path to OVA package (single file format). 3. Storage pool name where new domain needs to be created. API returns pointers to newly created domain that is not powered ON. Libvirt driver sanitizes the inputs and calls hypervisor specific driver callbacks to abstract OVA install implementation. --- include/libvirt/libvirt.h.in |3 +++ src/driver.h |5 src/libvirt.c| 58 ++ src/libvirt_public.syms |5 4 files changed, 71 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 09c89c5..ea342bc 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1919,6 +1919,9 @@ int virDomainMemoryPeek (virDomainPtr dom, */ virDomainPtrvirDomainDefineXML (virConnectPtr conn, const char *xml); +virDomainPtrvirDomainDefineOVA (virConnectPtr conn, + const char *ovapath, + const char *poolname); NACK, as I said with previous postings, this does not belong in libvirt APIs, it should be built as a layer above. Further this API design fundamentally flawed as it is assuming the hypervisor driver can access files that are on the libvirt application machine which is not the case in general. 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-java 1/9] Add StorageVol and DomainBlock resize methods
At Sat, 5 Jan 2013 12:48:18 +0100,
Wido den Hollander wrote:
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/jna/Libvirt.java |2 ++
1 file changed, 2 insertions(+)
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java
b/src/main/java/org/libvirt/jna/Libvirt.java
index e68d9ed..dbd8f6c 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -168,6 +168,7 @@ public interface Libvirt extends Library {
public int virDomainAttachDevice(DomainPointer virDomainPtr, String
deviceXML);
public int virDomainAttachDeviceFlags(DomainPointer virDomainPtr, String
deviceXML, int flags);
public int virDomainBlockStats(DomainPointer virDomainPtr, String path,
virDomainBlockStats stats, int size);
+public int virDomainBlockResize(DomainPointer virDomainPtr, String disk,
NativeLong size, int flags);
public int virDomainCoreDump(DomainPointer virDomainPtr, String to, int
flags);
public int virDomainCreate(DomainPointer virDomainPtr);
public int virDomainCreateWithFlags(DomainPointer virDomainPtr, int
flags);
@@ -313,6 +314,7 @@ public interface Libvirt extends Library {
public StorageVolPointer virStorageVolLookupByName(StoragePoolPointer
storagePoolPtr, String name);
public StorageVolPointer virStorageVolLookupByPath(ConnectionPointer
virConnectPtr, String path);
public int virStorageVolWipe(StorageVolPointer storageVolPtr, int flags);
+public int virStorageVolResize(StorageVolPointer storageVolPtr,
NativeLong capacity, int flags);
// Interface Methods
public int virInterfaceCreate(InterfacePointer virDevicePointer);
NACK since the wrapping is wrong.
The native type of the capacity parameter is unsigned long long.
When wrapping this type using JNA one should just use the Java long type.
See https://github.com/twall/jna/blob/master/www/Mappings.md
Btw, using the public modifier in interfaces is discouraged in Java
(see JLS 9.4). I'll push a patch shortly removing those modifiers. So,
please refrain from using those in new code.
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] [PATCH libvirt-java 9/9] Add resize flags to StorageVol
At Sat, 5 Jan 2013 12:48:26 +0100,
Wido den Hollander wrote:
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/StorageVol.java |7 +++
1 file changed, 7 insertions(+)
diff --git a/src/main/java/org/libvirt/StorageVol.java
b/src/main/java/org/libvirt/StorageVol.java
index 66e647f..9ea23c7 100644
--- a/src/main/java/org/libvirt/StorageVol.java
+++ b/src/main/java/org/libvirt/StorageVol.java
@@ -23,6 +23,13 @@ public class StorageVol {
static final int VIR_STORAGE_POOL_DELETE_ZEROED = 1;
}
+static final class ResizeFlags {
+/**
+ * Size is in bytes instead of KiB
+ */
+static final int VIR_DOMAIN_BLOCK_RESIZE_BYTES = (1 0);
+}
+
public static enum Type {
/**
NACK. According to the libvirt API, virStorageVolResizeFlags has these
possible values:
VIR_STORAGE_VOL_RESIZE_ALLOCATE = 1: force allocation of new size
VIR_STORAGE_VOL_RESIZE_DELTA = 2: size is relative to current
VIR_STORAGE_VOL_RESIZE_SHRINK= 4: allow decrease in capacity
You probably want to move the stanza above into Domain.java. Please
rename the class when you do this to BlockResizeFlags to be
consistant with the naming in libvirt.
Additionally, consensus was that we should drop the C prefixes from
enum values when wrapping them in Java. I will do this for existing
enums when I find the time, but for new enums we could just as well
avoid introducing them in the first place.
In this case the prefix of VIR_DOMAIN_BLOCK_RESIZE should be
removed. Usage would thus be
Domain.BlockResizeFlags.BYTES
instead of the repetitious
Domain.BlockResizeFlags.VIR_DOMAIN_BLOCK_RESIZE_BYTES
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] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if CONFIG_KVM is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
+if (kvm_enabled()) {
+kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
+}
+#endif
}
void host_cpuid(uint32_t function, uint32_t count,
--
1.7.11.7
--
Gleb.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Mon, Jan 07, 2013 at 09:42:36AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if CONFIG_KVM is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
Why not define KVM_FEATURE_PV_EOI unconditionally?
+if (kvm_enabled()) {
+kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
+}
+#endif
}
void host_cpuid(uint32_t function, uint32_t count,
--
1.7.11.7
--
Gleb.
--
Eduardo
--
Gleb.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 02/11] target-i386: Disable kvm_mmu_op by default on pc-1.4
On Sun, Jan 06, 2013 at 03:38:28PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:03PM -0200, Eduardo Habkost wrote:
The kvm_mmu_op feature was removed from the kernel since v3.3 (released
in March 2012), it was marked for removal since January 2011 and it's
slower than shadow or hardware assisted paging (see kernel commit
fb92045843). It doesn't make sense to keep it enabled by default.
Actually it was effectively removed Oct 1 2009 by a68a6a7282373. After 3
and a half years of not having it I think we can safely drop it without
trying to preserve it in older machine types.
Agreed. Especially considering that the check/enforce code for KVM flags
is currently broken. So probably people using pc-1.0, pc-1.1, pc-1.2 are
probably _not_ getting the kvm_mmu feature exposed to the guest.
Also, keeping it enabled by default would cause unnecessary hassle when
libvirt start using the enforce option.
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
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
I was planning to reverse the logic of the compat init functions and
make pc_init_pci_1_3() enable kvm_mmu_op and then call pc_init_pci_1_4()
instead. But that would require changing pc_init_pci_no_kvmclock() and
pc_init_isa() as well. So to keep the changes simple, I am keeping the
pattern used when pc_init_pci_1_3() was introduced, making
pc_init_pci_1_4() disable kvm_mmu_op and then call pc_init_pci_1_3().
Changes v2:
- Coding style fix
- Removed redundant comments above machine init functions
---
hw/pc_piix.c | 9 -
target-i386/cpu.c | 9 +
target-i386/cpu.h | 1 +
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 99747a7..a32af6a 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -217,6 +217,7 @@ static void pc_init1(MemoryRegion *system_memory,
}
}
+/* machine init function for pc-0.14 - pc-1.2 */
static void pc_init_pci(QEMUMachineInitArgs *args)
{
ram_addr_t ram_size = args-ram_size;
@@ -238,6 +239,12 @@ static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
pc_init_pci(args);
}
+static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
+{
+disable_kvm_mmu_op();
+pc_init_pci_1_3(args);
+}
+
static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
{
ram_addr_t ram_size = args-ram_size;
@@ -285,7 +292,7 @@ static QEMUMachine pc_machine_v1_4 = {
.name = pc-1.4,
.alias = pc,
.desc = Standard PC,
-.init = pc_init_pci_1_3,
+.init = pc_init_pci_1_4,
.max_cpus = 255,
.is_default = 1,
};
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e6435da..c83a566 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -158,6 +158,15 @@ void enable_kvm_pv_eoi(void)
#endif
}
+void disable_kvm_mmu_op(void)
+{
+#ifdef CONFIG_KVM
No need for ifdef here too.
Same case of the previous patch: KVM_FEATURE_MMU_OP is available only if
CONFIG_KVM is set.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java 2/9] Add resize method in StorageVol
At Sat, 5 Jan 2013 12:48:19 +0100,
Wido den Hollander wrote:
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/StorageVol.java | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/main/java/org/libvirt/StorageVol.java
b/src/main/java/org/libvirt/StorageVol.java
index 1bed6e1..66e647f 100644
--- a/src/main/java/org/libvirt/StorageVol.java
+++ b/src/main/java/org/libvirt/StorageVol.java
@@ -4,6 +4,8 @@ import org.libvirt.jna.Libvirt;
import org.libvirt.jna.StoragePoolPointer;
import org.libvirt.jna.StorageVolPointer;
import org.libvirt.jna.virStorageVolInfo;
+import com.sun.jna.Native;
This import is unused.
+import com.sun.jna.NativeLong;
/**
* An acutal storage bucket.
@@ -208,4 +210,21 @@ public class StorageVol {
processError();
return returnValue;
}
+
+/**
+ * Resize a volume
+ *
+ * @see a
href=http://www.libvirt.org/html/libvirt-libvirt.html#virStorageVolResize;Libvirt
Documentation/a
+ * @param capacity
+ *new capacity for volume
+ * @param flags
+ *flags for resizing, see libvirt API for exact flags
+ * @return 0 on success, or -1 on error
+ * @throws LibvirtException
+ */
+public int resize(NativeLong capacity, int flags) throws
LibvirtException {
+int returnValue = libvirt.virStorageVolResize(VSVP, capacity, flags);
+processError();
+return returnValue;
+}
JNA types should not be exposed to the public API, IMHO. So, using
NativeLong in the parameter list is not good.
And see my comment for 1/9.
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] [PATCH qom-cpu 05/11] target-i386: check/enforce: Fix CPUID leaf numbers on error messages
On Sun, Jan 06, 2013 at 04:12:54PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:06PM -0200, Eduardo Habkost wrote:
The -cpu check/enforce warnings are printing incorrect information about the
missing flags. There are no feature flags on CPUID leaves 0 and 0x8000,
but
there were references to 0 and 0x8000 in the table at
kvm_check_features_against_host().
This changes the model_features_t struct to contain the register number as
well, so the error messages print the correct CPUID leaf+register
information,
instead of wrong CPUID leaf numbers.
This also changes the format of the error messages, so they follow the
CPUID.leaf.register.name [bit offset] convention used on Intel
documentation. Example output:
$ qemu-system-x86_64 -machine pc-1.0,accel=kvm -cpu
Opteron_G4,+ia64,enforce
warning: host doesn't support requested feature: CPUID.01H:EDX.ia64
[bit 30]
warning: host doesn't support requested feature: CPUID.01H:ECX.xsave
[bit 26]
warning: host doesn't support requested feature: CPUID.01H:ECX.avx [bit
28]
warning: host doesn't support requested feature:
CPUID.8001H:ECX.abm [bit 5]
warning: host doesn't support requested feature:
CPUID.8001H:ECX.sse4a [bit 6]
warning: host doesn't support requested feature:
CPUID.8001H:ECX.misalignsse [bit 7]
warning: host doesn't support requested feature:
CPUID.8001H:ECX.3dnowprefetch [bit 8]
warning: host doesn't support requested feature:
CPUID.8001H:ECX.xop [bit 11]
warning: host doesn't support requested feature:
CPUID.8001H:ECX.fma4 [bit 16]
Unable to find x86 CPU definition
$
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
Reviewed-by: Gleb Natapov g...@redhat.com
But see the question below.
---
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: k...@vger.kernel.org
Changes v2:
- Coding style fixes
- Add assert() for invalid register numbers on
unavailable_host_feature()
---
target-i386/cpu.c | 42 +-
target-i386/cpu.h | 3 +++
2 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e916ae0..c3e5db8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -124,6 +124,25 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
};
+const char *get_register_name_32(unsigned int reg)
+{
+static const char *reg_names[CPU_NB_REGS32] = {
+[R_EAX] = EAX,
+[R_ECX] = ECX,
+[R_EDX] = EDX,
+[R_EBX] = EBX,
+[R_ESP] = ESP,
+[R_EBP] = EBP,
+[R_ESI] = ESI,
+[R_EDI] = EDI,
+};
+
+if (reg CPU_NB_REGS32) {
+return NULL;
+}
+return reg_names[reg];
+}
+
/* collects per-function cpuid data
*/
typedef struct model_features_t {
@@ -132,7 +151,8 @@ typedef struct model_features_t {
uint32_t check_feat;
const char **flag_names;
uint32_t cpuid;
-} model_features_t;
+int reg;
+} model_features_t;
int check_cpuid = 0;
int enforce_cpuid = 0;
@@ -923,10 +943,13 @@ static int unavailable_host_feature(struct
model_features_t *f, uint32_t mask)
for (i = 0; i 32; ++i)
if (1 i mask) {
-fprintf(stderr, warning: host cpuid %04x_%04x lacks requested
- flag '%s' [0x%08x]\n,
-f-cpuid 16, f-cpuid 0x,
-f-flag_names[i] ? f-flag_names[i] : [reserved], mask);
+const char *reg = get_register_name_32(f-reg);
+assert(reg);
+fprintf(stderr, warning: host doesn't support requested
feature:
+CPUID.%02XH:%s%s%s [bit %d]\n,
+f-cpuid, reg,
+f-flag_names[i] ? . : ,
+f-flag_names[i] ? f-flag_names[i] : , i);
break;
}
return 0;
@@ -945,13 +968,14 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
int rv, i;
struct model_features_t ft[] = {
{guest_def-features, host_def.features,
-~0, feature_name, 0x},
+~0, feature_name, 0x0001, R_EDX},
{guest_def-ext_features, host_def.ext_features,
-~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x0001},
+~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x0001, R_ECX},
{guest_def-ext2_features, host_def.ext2_features,
-~PPRO_FEATURES, ext2_feature_name, 0x8000},
+~PPRO_FEATURES, ext2_feature_name, 0x8001, R_EDX},
{guest_def-ext3_features, host_def.ext3_features,
-~CPUID_EXT3_SVM, ext3_feature_name, 0x8001}};
+~CPUID_EXT3_SVM,
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 10/11] target-i386: Call kvm_check_features_against_host() only if CONFIG_KVM is set
On Sun, Jan 06, 2013 at 04:27:19PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:11PM -0200, Eduardo Habkost wrote:
This will be necessary once kvm_check_features_against_host() starts
using KVM-specific definitions (so it won't compile anymore if
CONFIG_KVM is not set).
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1c3c7e1..876b0f6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -936,6 +936,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
#endif /* CONFIG_KVM */
}
+#ifdef CONFIG_KVM
static int unavailable_host_feature(struct model_features_t *f, uint32_t
mask)
{
int i;
@@ -987,6 +988,7 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
}
return rv;
}
+#endif
static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void
*opaque,
const char *name, Error **errp)
@@ -1410,10 +1412,12 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-kvm_features = ~minus_kvm_features;
x86_cpu_def-svm_features = ~minus_svm_features;
x86_cpu_def-cpuid_7_0_ebx_features = ~minus_7_0_ebx_features;
+#ifdef CONFIG_KVM
if (check_cpuid kvm_enabled()) {
if (kvm_check_features_against_host(x86_cpu_def) enforce_cpuid)
goto error;
}
+#endif
Provide kvm_check_features_against_host() stub if !CONFIG_KVM and drop
ifdef here.
I will do. Igor probably will have to change his target-i386: move
kvm_check_features_against_host() check to realize time patch to use
the same approach, too.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 11/11] target-i386: check/enforce: Check all feature words
On Sun, Jan 06, 2013 at 04:35:51PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:12PM -0200, Eduardo Habkost wrote:
This adds the following feature words to the list of flags to be checked
by kvm_check_features_against_host():
- cpuid_7_0_ebx_features
- ext4_features
- kvm_features
- svm_features
This will ensure the enforce flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.
This patch may cause existing configurations where enforce wasn't
preventing QEMU from being started to abort QEMU. But that's exactly the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the enforce code.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: k...@vger.kernel.org
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
CCing libvirt people, as this is directly related to the planned usage
of the enforce flag by libvirt.
The libvirt team probably has a problem in their hands: libvirt should
use enforce to make sure all requested flags are making their way into
the guest (so the resulting CPU is always the same, on any host), but
users may have existing working configurations where a flag is not
supported by the guest and the user really doesn't care about it. Those
configurations will necessarily break when libvirt starts using
enforce.
One example where it may cause trouble for common setups: pc-1.3 wants
the kvm_pv_eoi flag enabled by default (so enforce will make sure it
is enabled), but the user may have an existing VM running on a host
without pv_eoi support. That setup is unsafe today because
live-migration between different host kernel versions may enable/disable
pv_eoi silently (that's why we need the enforce flag to be used by
libvirt), but the user probably would like to be able to live-migrate
that VM anyway (and have libvirt to just do the right thing).
One possible solution to libvirt is to use enforce only on newer
machine-types, so existing machines with older machine-types will keep
the unsafe host-dependent-ABI behavior, but at least would keep
live-migration working in case the user is careful.
I really don't know what the libvirt team prefers, but that's the
situation today. 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.
Changes v2:
- Coding style fix
---
target-i386/cpu.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 876b0f6..52727ad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -955,8 +955,9 @@ static int unavailable_host_feature(struct
model_features_t *f, uint32_t mask)
return 0;
}
-/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
*
* This function may be called only if KVM is enabled.
*/
@@ -973,7 +974,15 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
{guest_def-ext2_features, host_def.ext2_features,
ext2_feature_name, 0x8001, R_EDX},
{guest_def-ext3_features, host_def.ext3_features,
-ext3_feature_name, 0x8001, R_ECX}
+ext3_feature_name, 0x8001, R_ECX},
+{guest_def-ext4_features, host_def.ext4_features,
+NULL, 0xC001, R_EDX},
Since there is not name array for ext4_features they cannot be added or
removed on the command line hence no need to check them, no?
In theory, yes. But it won't hurt to check it, and it will be useful to
unify the list of feature words in a single place, so we can be sure the
checking/filtering/setting code at
kvm_check_features_against_host()/kvm_filter_features_for_host()/kvm_cpu_fill_host(),
will all check/filter/set exactly the same feature words.
+{guest_def-cpuid_7_0_ebx_features,
host_def.cpuid_7_0_ebx_features,
+cpuid_7_0_ebx_feature_name, 7, R_EBX},
+{guest_def-svm_features, host_def.svm_features,
+svm_feature_name, 0x800A, R_EDX},
+{guest_def-kvm_features, host_def.kvm_features,
+kvm_feature_name, KVM_CPUID_FEATURES, R_EAX},
};
assert(kvm_enabled());
--
1.7.11.7
--
Gleb.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: fix leak in virStorageBackendLogicalMakeVol
Use regfree instead of VIR_FREE. --- src/storage/storage_backend_logical.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 2734689..bd902fe 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -264,7 +264,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, cleanup: VIR_FREE(regex); -VIR_FREE(reg); +regfree(reg); VIR_FREE(vars); if (is_new_vol (ret == -1)) virStorageVolDefFree(vol); -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 11/11] target-i386: check/enforce: Check all feature words
On Mon, Jan 07, 2013 at 10:06:21AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 04:35:51PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:12PM -0200, Eduardo Habkost wrote:
This adds the following feature words to the list of flags to be checked
by kvm_check_features_against_host():
- cpuid_7_0_ebx_features
- ext4_features
- kvm_features
- svm_features
This will ensure the enforce flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.
This patch may cause existing configurations where enforce wasn't
preventing QEMU from being started to abort QEMU. But that's exactly the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the enforce code.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: k...@vger.kernel.org
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
CCing libvirt people, as this is directly related to the planned usage
of the enforce flag by libvirt.
The libvirt team probably has a problem in their hands: libvirt should
use enforce to make sure all requested flags are making their way into
the guest (so the resulting CPU is always the same, on any host), but
users may have existing working configurations where a flag is not
supported by the guest and the user really doesn't care about it. Those
configurations will necessarily break when libvirt starts using
enforce.
One example where it may cause trouble for common setups: pc-1.3 wants
the kvm_pv_eoi flag enabled by default (so enforce will make sure it
is enabled), but the user may have an existing VM running on a host
without pv_eoi support. That setup is unsafe today because
live-migration between different host kernel versions may enable/disable
pv_eoi silently (that's why we need the enforce flag to be used by
libvirt), but the user probably would like to be able to live-migrate
that VM anyway (and have libvirt to just do the right thing).
One possible solution to libvirt is to use enforce only on newer
machine-types, so existing machines with older machine-types will keep
the unsafe host-dependent-ABI behavior, but at least would keep
live-migration working in case the user is careful.
I really don't know what the libvirt team prefers, but that's the
situation today. 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.
Changes v2:
- Coding style fix
---
target-i386/cpu.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 876b0f6..52727ad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -955,8 +955,9 @@ static int unavailable_host_feature(struct
model_features_t *f, uint32_t mask)
return 0;
}
-/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
*
* This function may be called only if KVM is enabled.
*/
@@ -973,7 +974,15 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
{guest_def-ext2_features, host_def.ext2_features,
ext2_feature_name, 0x8001, R_EDX},
{guest_def-ext3_features, host_def.ext3_features,
-ext3_feature_name, 0x8001, R_ECX}
+ext3_feature_name, 0x8001, R_ECX},
+{guest_def-ext4_features, host_def.ext4_features,
+NULL, 0xC001, R_EDX},
Since there is not name array for ext4_features they cannot be added or
removed on the command line hence no need to check them, no?
In theory, yes. But it won't hurt to check it, and it will be useful to
unify the list of feature words in a single place, so we can be sure the
checking/filtering/setting code at
kvm_check_features_against_host()/kvm_filter_features_for_host()/kvm_cpu_fill_host(),
will all check/filter/set exactly the same feature words.
May be add a name array for the leaf? :)
--
Gleb.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Mon, Jan 07, 2013 at 01:42:53PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 09:42:36AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if CONFIG_KVM is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
Why not define KVM_FEATURE_PV_EOI unconditionally?
It comes from the KVM kernel headers, that are included only if
CONFIG_KVM is set, and probably won't even compile in non-Linux systems.
I have a dejavu feeling. I believe we had this exact problem before,
maybe about some other #defines that come from the Linux KVM headers and
won't be available in non-Linux systems.
+if (kvm_enabled()) {
+kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
+}
+#endif
}
void host_cpuid(uint32_t function, uint32_t count,
--
1.7.11.7
--
Gleb.
--
Eduardo
--
Gleb.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Mon, Jan 07, 2013 at 10:09:24AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 01:42:53PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 09:42:36AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if CONFIG_KVM is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
Why not define KVM_FEATURE_PV_EOI unconditionally?
It comes from the KVM kernel headers, that are included only if
CONFIG_KVM is set, and probably won't even compile in non-Linux systems.
I have a dejavu feeling. I believe we had this exact problem before,
maybe about some other #defines that come from the Linux KVM headers and
won't be available in non-Linux systems.
It is better to hide all KVM related differences somewhere in the
headers where no one sees them instead of sprinkle them all over the
code. We can put those defines in include/sysemu/kvm.h in !CONFIG_KVM
part. Or have one ifdef CONFIG_KVM at the beginning of the file and
define enable_kvm_pv_eoi() there and provide empty stub otherwise.
--
Gleb.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 11/11] target-i386: check/enforce: Check all feature words
On Mon, Jan 07, 2013 at 02:06:38PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 10:06:21AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 04:35:51PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:12PM -0200, Eduardo Habkost wrote:
This adds the following feature words to the list of flags to be checked
by kvm_check_features_against_host():
- cpuid_7_0_ebx_features
- ext4_features
- kvm_features
- svm_features
This will ensure the enforce flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.
This patch may cause existing configurations where enforce wasn't
preventing QEMU from being started to abort QEMU. But that's exactly the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the enforce code.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: k...@vger.kernel.org
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
CCing libvirt people, as this is directly related to the planned usage
of the enforce flag by libvirt.
The libvirt team probably has a problem in their hands: libvirt should
use enforce to make sure all requested flags are making their way into
the guest (so the resulting CPU is always the same, on any host), but
users may have existing working configurations where a flag is not
supported by the guest and the user really doesn't care about it. Those
configurations will necessarily break when libvirt starts using
enforce.
One example where it may cause trouble for common setups: pc-1.3 wants
the kvm_pv_eoi flag enabled by default (so enforce will make sure it
is enabled), but the user may have an existing VM running on a host
without pv_eoi support. That setup is unsafe today because
live-migration between different host kernel versions may enable/disable
pv_eoi silently (that's why we need the enforce flag to be used by
libvirt), but the user probably would like to be able to live-migrate
that VM anyway (and have libvirt to just do the right thing).
One possible solution to libvirt is to use enforce only on newer
machine-types, so existing machines with older machine-types will keep
the unsafe host-dependent-ABI behavior, but at least would keep
live-migration working in case the user is careful.
I really don't know what the libvirt team prefers, but that's the
situation today. 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.
Changes v2:
- Coding style fix
---
target-i386/cpu.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 876b0f6..52727ad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -955,8 +955,9 @@ static int unavailable_host_feature(struct
model_features_t *f, uint32_t mask)
return 0;
}
-/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero
otherwise.
*
* This function may be called only if KVM is enabled.
*/
@@ -973,7 +974,15 @@ static int
kvm_check_features_against_host(x86_def_t *guest_def)
{guest_def-ext2_features, host_def.ext2_features,
ext2_feature_name, 0x8001, R_EDX},
{guest_def-ext3_features, host_def.ext3_features,
-ext3_feature_name, 0x8001, R_ECX}
+ext3_feature_name, 0x8001, R_ECX},
+{guest_def-ext4_features, host_def.ext4_features,
+NULL, 0xC001, R_EDX},
Since there is not name array for ext4_features they cannot be added or
removed on the command line hence no need to check them, no?
In theory, yes. But it won't hurt to check it, and it will be useful to
unify the list of feature words in a single place, so we can be sure the
checking/filtering/setting code at
kvm_check_features_against_host()/kvm_filter_features_for_host()/kvm_cpu_fill_host(),
will all check/filter/set exactly the same feature words.
May be add a name array for the leaf? :)
If anybody find reliable documentation about the 0xC001 CPUID bits,
I would happily do it. :-)
While we don't have the docs and feature names, I still believe that
having the complete list of feature words in the
kvm_check_features_against_host() code will save us trouble later,
Re: [libvirt] [PATCH qom-cpu 11/11] target-i386: check/enforce: Check all feature words
On Mon, Jan 07, 2013 at 10:19:15AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 02:06:38PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 10:06:21AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 04:35:51PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:12PM -0200, Eduardo Habkost wrote:
This adds the following feature words to the list of flags to be
checked
by kvm_check_features_against_host():
- cpuid_7_0_ebx_features
- ext4_features
- kvm_features
- svm_features
This will ensure the enforce flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.
This patch may cause existing configurations where enforce wasn't
preventing QEMU from being started to abort QEMU. But that's exactly
the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the enforce code.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: k...@vger.kernel.org
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
CCing libvirt people, as this is directly related to the planned usage
of the enforce flag by libvirt.
The libvirt team probably has a problem in their hands: libvirt should
use enforce to make sure all requested flags are making their way
into
the guest (so the resulting CPU is always the same, on any host), but
users may have existing working configurations where a flag is not
supported by the guest and the user really doesn't care about it.
Those
configurations will necessarily break when libvirt starts using
enforce.
One example where it may cause trouble for common setups: pc-1.3 wants
the kvm_pv_eoi flag enabled by default (so enforce will make sure it
is enabled), but the user may have an existing VM running on a host
without pv_eoi support. That setup is unsafe today because
live-migration between different host kernel versions may
enable/disable
pv_eoi silently (that's why we need the enforce flag to be used by
libvirt), but the user probably would like to be able to live-migrate
that VM anyway (and have libvirt to just do the right thing).
One possible solution to libvirt is to use enforce only on newer
machine-types, so existing machines with older machine-types will keep
the unsafe host-dependent-ABI behavior, but at least would keep
live-migration working in case the user is careful.
I really don't know what the libvirt team prefers, but that's the
situation today. 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.
Changes v2:
- Coding style fix
---
target-i386/cpu.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 876b0f6..52727ad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -955,8 +955,9 @@ static int unavailable_host_feature(struct
model_features_t *f, uint32_t mask)
return 0;
}
-/* best effort attempt to inform user requested cpu flags aren't
making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero
otherwise.
*
* This function may be called only if KVM is enabled.
*/
@@ -973,7 +974,15 @@ static int
kvm_check_features_against_host(x86_def_t *guest_def)
{guest_def-ext2_features, host_def.ext2_features,
ext2_feature_name, 0x8001, R_EDX},
{guest_def-ext3_features, host_def.ext3_features,
-ext3_feature_name, 0x8001, R_ECX}
+ext3_feature_name, 0x8001, R_ECX},
+{guest_def-ext4_features, host_def.ext4_features,
+NULL, 0xC001, R_EDX},
Since there is not name array for ext4_features they cannot be added or
removed on the command line hence no need to check them, no?
In theory, yes. But it won't hurt to check it, and it will be useful to
unify the list of feature words in a single place, so we can be sure the
checking/filtering/setting code at
kvm_check_features_against_host()/kvm_filter_features_for_host()/kvm_cpu_fill_host(),
will all check/filter/set exactly the same feature words.
May be add a name array for the leaf? :)
If anybody find reliable documentation about the 0xC001 CPUID bits,
I would
Re: [libvirt] [PATCH] storage: fix leak in virStorageBackendLogicalMakeVol
On 01/07/13 13:05, Ján Tomko wrote: Use regfree instead of VIR_FREE. --- src/storage/storage_backend_logical.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Mon, Jan 07, 2013 at 02:15:59PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 10:09:24AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 01:42:53PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 09:42:36AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if CONFIG_KVM is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
Why not define KVM_FEATURE_PV_EOI unconditionally?
It comes from the KVM kernel headers, that are included only if
CONFIG_KVM is set, and probably won't even compile in non-Linux systems.
I have a dejavu feeling. I believe we had this exact problem before,
maybe about some other #defines that come from the Linux KVM headers and
won't be available in non-Linux systems.
It is better to hide all KVM related differences somewhere in the
headers where no one sees them instead of sprinkle them all over the
code. We can put those defines in include/sysemu/kvm.h in !CONFIG_KVM
part. Or have one ifdef CONFIG_KVM at the beginning of the file and
define enable_kvm_pv_eoi() there and provide empty stub otherwise.
If we had an empty enable_kvm_pv_eoi() stub, we would need an #ifdef
around the real implementation. I mean, I don't think this:
#ifdef CONFIG_KVM
int enable_kvm_pv_eoi() {
[...]
}
#endif
is any better than this:
int enable_kvm_pv_eoi() {
#ifdef CONFIG_KVM
[...]
#endif
}
So this is probably a good reason to duplicate the KVM_FEATURE_*
#defines in the QEMU code, instead?
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Mon, Jan 07, 2013 at 02:33:25PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 10:30:40AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 02:15:59PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 10:09:24AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 01:42:53PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 09:42:36AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if
CONFIG_KVM is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
Why not define KVM_FEATURE_PV_EOI unconditionally?
It comes from the KVM kernel headers, that are included only if
CONFIG_KVM is set, and probably won't even compile in non-Linux systems.
I have a dejavu feeling. I believe we had this exact problem before,
maybe about some other #defines that come from the Linux KVM headers and
won't be available in non-Linux systems.
It is better to hide all KVM related differences somewhere in the
headers where no one sees them instead of sprinkle them all over the
code. We can put those defines in include/sysemu/kvm.h in !CONFIG_KVM
part. Or have one ifdef CONFIG_KVM at the beginning of the file and
define enable_kvm_pv_eoi() there and provide empty stub otherwise.
If we had an empty enable_kvm_pv_eoi() stub, we would need an #ifdef
around the real implementation. I mean, I don't think this:
#ifdef CONFIG_KVM
int enable_kvm_pv_eoi() {
[...]
}
#endif
You already have #ifdef CONFIG_KVM just above enable_kvm_pv_eoi(). Put
everything KVM related there instead of adding #ifdef CONFIG_KVM all
over the file.
But it also creates the need to write a separate stub function somewhere
else, while we could have a ready-to-use stub function automatically by
simply #ifdefing the whole function body. But anyway: this won't matter
if we choose the duplicate/fake #defines approach mentioned below.
is any better than this:
int enable_kvm_pv_eoi() {
#ifdef CONFIG_KVM
[...]
#endif
}
So this is probably a good reason to duplicate the
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 10/11] target-i386: Call kvm_check_features_against_host() only if CONFIG_KVM is set
On Mon, 7 Jan 2013 10:00:09 -0200
Eduardo Habkost ehabk...@redhat.com wrote:
On Sun, Jan 06, 2013 at 04:27:19PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:11PM -0200, Eduardo Habkost wrote:
This will be necessary once kvm_check_features_against_host() starts
using KVM-specific definitions (so it won't compile anymore if
CONFIG_KVM is not set).
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1c3c7e1..876b0f6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -936,6 +936,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
#endif /* CONFIG_KVM */
}
+#ifdef CONFIG_KVM
static int unavailable_host_feature(struct model_features_t *f, uint32_t
mask)
{
int i;
@@ -987,6 +988,7 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
}
return rv;
}
+#endif
static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void
*opaque,
const char *name, Error **errp)
@@ -1410,10 +1412,12 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-kvm_features = ~minus_kvm_features;
x86_cpu_def-svm_features = ~minus_svm_features;
x86_cpu_def-cpuid_7_0_ebx_features = ~minus_7_0_ebx_features;
+#ifdef CONFIG_KVM
if (check_cpuid kvm_enabled()) {
if (kvm_check_features_against_host(x86_cpu_def)
enforce_cpuid)
goto error;
}
+#endif
Provide kvm_check_features_against_host() stub if !CONFIG_KVM and drop
ifdef here.
I will do. Igor probably will have to change his target-i386: move
kvm_check_features_against_host() check to realize time patch to use
the same approach, too.
Gleb,
Why do stub here? As result we will be adding more ifdef-s just in other
places. Currently kvm_cpu_fill_host(), unavailable_host_feature() and
kvm_check_features_against_host() are bundled together in cpu.c so we could
instead ifdef whole block. Like here:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg146536.html
For me code looks more readable with ifdef here, if we have stub, a reader
would have to look at kvm_check_features_against_host() body to see if it does
anything.
--
Eduardo
--
Regards,
Igor
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH qom-cpu 01/11] target-i386: Don't set any KVM flag by default if KVM is disabled
On Mon, Jan 07, 2013 at 10:30:40AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 02:15:59PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 10:09:24AM -0200, Eduardo Habkost wrote:
On Mon, Jan 07, 2013 at 01:42:53PM +0200, Gleb Natapov wrote:
On Mon, Jan 07, 2013 at 09:42:36AM -0200, Eduardo Habkost wrote:
On Sun, Jan 06, 2013 at 01:32:34PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:02PM -0200, Eduardo Habkost 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
CONFIG_KVM and 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.
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
---
target-i386/cpu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 82685dc..e6435da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -145,15 +145,17 @@ 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;
+#ifdef CONFIG_KVM
You do not need ifdef here.
We need it because KVM_FEATURE_PV_EOI is available only if CONFIG_KVM
is
set.
I could also write it as:
if (kvm_enabled()) {
#ifdef CONFIG_KVM
kvm_default_features |= (1UL KVM_FEATURE_PV_EOI);
#endif
}
But I find it less readable.
Why not define KVM_FEATURE_PV_EOI unconditionally?
It comes from the KVM kernel headers, that are included only if
CONFIG_KVM is set, and probably won't even compile in non-Linux systems.
I have a dejavu feeling. I believe we had this exact problem before,
maybe about some other #defines that come from the Linux KVM headers and
won't be available in non-Linux systems.
It is better to hide all KVM related differences somewhere in the
headers where no one sees them instead of sprinkle them all over the
code. We can put those defines in include/sysemu/kvm.h in !CONFIG_KVM
part. Or have one ifdef CONFIG_KVM at the beginning of the file and
define enable_kvm_pv_eoi() there and provide empty stub otherwise.
If we had an empty enable_kvm_pv_eoi() stub, we would need an #ifdef
around the real implementation. I mean, I don't think this:
#ifdef CONFIG_KVM
int enable_kvm_pv_eoi() {
[...]
}
#endif
You already have #ifdef CONFIG_KVM just above enable_kvm_pv_eoi(). Put
everything KVM related there instead of adding #ifdef CONFIG_KVM all
over the file.
is any better than this:
int enable_kvm_pv_eoi() {
#ifdef CONFIG_KVM
[...]
#endif
}
So this is probably a good reason to duplicate the KVM_FEATURE_*
#defines in the QEMU code, instead?
Not even duplicate, they can be fake just to keep compiler happy.
--
Gleb.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 10/11] target-i386: Call kvm_check_features_against_host() only if CONFIG_KVM is set
On Mon, Jan 07, 2013 at 02:15:14PM +0100, Igor Mammedov wrote:
On Mon, 7 Jan 2013 10:00:09 -0200
Eduardo Habkost ehabk...@redhat.com wrote:
On Sun, Jan 06, 2013 at 04:27:19PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:11PM -0200, Eduardo Habkost wrote:
This will be necessary once kvm_check_features_against_host() starts
using KVM-specific definitions (so it won't compile anymore if
CONFIG_KVM is not set).
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1c3c7e1..876b0f6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -936,6 +936,7 @@ static void kvm_cpu_fill_host(x86_def_t
*x86_cpu_def)
#endif /* CONFIG_KVM */
}
+#ifdef CONFIG_KVM
static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
{
int i;
@@ -987,6 +988,7 @@ static int
kvm_check_features_against_host(x86_def_t *guest_def)
}
return rv;
}
+#endif
static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void
*opaque,
const char *name, Error
**errp)
@@ -1410,10 +1412,12 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-kvm_features = ~minus_kvm_features;
x86_cpu_def-svm_features = ~minus_svm_features;
x86_cpu_def-cpuid_7_0_ebx_features = ~minus_7_0_ebx_features;
+#ifdef CONFIG_KVM
if (check_cpuid kvm_enabled()) {
if (kvm_check_features_against_host(x86_cpu_def)
enforce_cpuid)
goto error;
}
+#endif
Provide kvm_check_features_against_host() stub if !CONFIG_KVM and drop
ifdef here.
I will do. Igor probably will have to change his target-i386: move
kvm_check_features_against_host() check to realize time patch to use
the same approach, too.
Gleb,
Why do stub here? As result we will be adding more ifdef-s just in other
places. Currently kvm_cpu_fill_host(), unavailable_host_feature() and
kvm_check_features_against_host() are bundled together in cpu.c so we could
instead ifdef whole block. Like here:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg146536.html
For me code looks more readable with ifdef here, if we have stub, a reader
would have to look at kvm_check_features_against_host() body to see if it does
anything.
If CONFIG_KVM is not set, kvm_enabled() is always zero, so the function
would never be called, so I find the ifdef-less code more readable and
obvious.
What I don't know is if we should do this:
#ifdef CONFIG_KVM
static int kvm_check_features_against_host(...)
{
/* real implementation here */
}
static int kvm_do_something_else(...)
{
/* real implementation here */
}
/* Other kvm_* functions here */
#else
static int kvm_check_features_against_host(...)
{
}
static int kvm_do_something_else(...)
{
}
/* Other kvm_* stubs here */
#endif /* CONFIG_KVM */
Or this:
static int kvm_check_features_against_host(...)
{
#ifdef CONFIG_KVM
/* real implementation here */
#endif /* CONFIG_KVM */
}
static int kvm_do_something_else(...)
{
#ifdef CONFIG_KVM
/* real implementation here */
#endif /* CONFIG_KVM */
}
I believe the latter is better, but based on Gleb's comments about
enable_kvm_pv_eoi(), he seems to prefer the former.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 10/11] target-i386: Call kvm_check_features_against_host() only if CONFIG_KVM is set
On Mon, Jan 07, 2013 at 02:15:14PM +0100, Igor Mammedov wrote:
On Mon, 7 Jan 2013 10:00:09 -0200
Eduardo Habkost ehabk...@redhat.com wrote:
On Sun, Jan 06, 2013 at 04:27:19PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:11PM -0200, Eduardo Habkost wrote:
This will be necessary once kvm_check_features_against_host() starts
using KVM-specific definitions (so it won't compile anymore if
CONFIG_KVM is not set).
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1c3c7e1..876b0f6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -936,6 +936,7 @@ static void kvm_cpu_fill_host(x86_def_t
*x86_cpu_def)
#endif /* CONFIG_KVM */
}
+#ifdef CONFIG_KVM
static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
{
int i;
@@ -987,6 +988,7 @@ static int
kvm_check_features_against_host(x86_def_t *guest_def)
}
return rv;
}
+#endif
static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void
*opaque,
const char *name, Error
**errp)
@@ -1410,10 +1412,12 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-kvm_features = ~minus_kvm_features;
x86_cpu_def-svm_features = ~minus_svm_features;
x86_cpu_def-cpuid_7_0_ebx_features = ~minus_7_0_ebx_features;
+#ifdef CONFIG_KVM
if (check_cpuid kvm_enabled()) {
if (kvm_check_features_against_host(x86_cpu_def)
enforce_cpuid)
goto error;
}
+#endif
Provide kvm_check_features_against_host() stub if !CONFIG_KVM and drop
ifdef here.
I will do. Igor probably will have to change his target-i386: move
kvm_check_features_against_host() check to realize time patch to use
the same approach, too.
Gleb,
Why do stub here? As result we will be adding more ifdef-s just in other
places. Currently kvm_cpu_fill_host(), unavailable_host_feature() and
Why will we be adding more ifdef-s in other places?
kvm_check_features_against_host() are bundled together in cpu.c so we could
instead ifdef whole block. Like here:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg146536.html
That's fine, but you can avoid things like:
if (kvm_enabled() name strcmp(name, host) == 0) {
+#ifdef CONFIG_KVM
kvm_cpu_fill_host(x86_cpu_def);
+#endif
in your patch by providing stub for kvm_cpu_fill_host() for !CONFIG_KVM
case. This is common practice really. Avoid ifdefs in the code.
For me code looks more readable with ifdef here, if we have stub, a reader
would have to look at kvm_check_features_against_host() body to see if it does
anything.
If reader cares about kvm it has to anyway. If he does not, there is
friendly kvm_enabled() (which is stub in case of !CONFIG_KVM BTW) to
tell him that he does not care. No need additional ifdef there.
--
Gleb.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: Add a hash table for the shared disks
On 2013年01月04日 22:30, Eric Blake wrote: On 01/03/2013 08:35 PM, Osier Yang wrote: ...when you could just use a single int value comprising the combination of major and minor as the hash key, if only you had used Sounds good, how about something like 805516, where 8 is major, and 16 is the minor? It should be small enough to be not overflowing. In fact, we already know of a single integer value large enough to contain both major and minor at the same time: the original st_rdev (of type dev_t) from stat(). We could simply add 'verify(sizeof(dev_t)= sizeof(void*))', then reconstruct the st_rdev value by undoing the decomposition done by the major() and minor() macros. Or, you could even tweak patch 1/6 to return st_rdev as a single value instead of major/minor as two separate values. Good, I will create a follow up patch, but later than pushing this set. Thanks, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java 3/9] Add virDomainBlockResize to Domain methods
At Sat, 5 Jan 2013 12:48:20 +0100,
Wido den Hollander wrote:
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/Domain.java | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index fc1f665..4b4c572 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -193,6 +193,24 @@ public class Domain {
}
/**
+ * Resize a block device of domain while the domain is running.
+ *
+ * @param disk
+ * path to the block image, or shorthand (like vda)
+ * @param size
+ * the new size of the block devices
+ * @param flags
+ * when set to 1, units of size is in bytes instead of
KiloBytes
+ * @return 0 on succes, -1 on error
This is not true as this method will throw an exception on error, it
should never return -1.
Just use return type void instead. Either it will work or throw an exception.
+ * @throws LibvirtException
+ */
+public int blockResize(String disk, NativeLong size, int flags) throws
LibvirtException {
+int returnValue = libvirt.virDomainBlockResize(VDP, disk, size,
flags);
+processError();
+return returnValue;
+}
Same comment applies to NativeLong as for patch 2/9.
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] [PATCH libvirt-java 4/9] Add the option to pass flags for snapshotCreateXML
At Sat, 5 Jan 2013 12:48:21 +0100,
Wido den Hollander wrote:
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/Domain.java | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index 4b4c572..e0be43d 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -997,18 +997,20 @@ public class Domain {
/**
* Creates a new snapshot of a domain based on the snapshot xml
contained in
- * xmlDesc.
+ * xmlDesc with the option to pass flags
Um, this sounds a bit odd to me, but I'm no native speaker.
Otherwise, looks good. ACK
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] [PATCH libvirt-java 5/9] Implement virDomainUndefineFlags
At Sat, 5 Jan 2013 12:48:22 +0100,
Wido den Hollander wrote:
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/Domain.java | 15 +++
src/main/java/org/libvirt/jna/Libvirt.java |1 +
2 files changed, 16 insertions(+)
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index e0be43d..d393960 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -1134,6 +1134,21 @@ public class Domain {
}
/**
+ * undefines this domain but does not stop if it it is running. With
flags option
Undefines (capital u)
s/if it/it if/
+ * @see a
href=http://libvirt.org/html/libvirt-libvirt.html#virDomainUndefineFlags;Libvirt
Documentation/a
+ * @param flags
+ *flags for undefining the domain. See
virDomainUndefineFlagsValues for more information
+ * @return 0 on success, -1 on error
Exception on error, never -1. Just use return type void.
+public int undefineFlags(int flags) throws LibvirtException {
Should we take advantage of Java's method overloading capability here?
I propose to divert from the mapping rules and just overload the
undefine method with an additional parameter.
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java
b/src/main/java/org/libvirt/jna/Libvirt.java
index dbd8f6c..2bbc8c3 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -228,6 +228,7 @@ public interface Libvirt extends Library {
public int virDomainSuspend(DomainPointer virDomainPtr);
public int virDomainUpdateDeviceFlags(DomainPointer virDomainPtr, String
xml, int flags);
public int virDomainUndefine(DomainPointer virDomainPtr);
+public int virDomainUndefineFlags(DomainPointer virDomainPtr, int flags);
Please drop the public modifier.
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] [PATCH 0/6 v9] Unprivileged SG_IO support
On 2013年01月02日 22:37, Osier Yang wrote: As a result of RFC [1], this implements the unprivleged SG_IO support. 1/6 and 2/6 are already acked. v8 - v9: * Just rebasing. v7 - v8: * Change the XML tag name from cdbfilter to sgio, and to leave enough room for future values, the values of sgio are filtered and unfiltered now. v6 - v7: * No restoring of unpriv_sgio per Daniel's thought. * Use major:minor as the hash key per Jirka's suggestion. Osier Yang (6): util: Prepare helpers for unpriv_sgio setting qemu: Add a hash table for the shared disks docs: Add docs and rng schema for new XML tag sgio conf: Parse and format the new XML qemu: set unpriv_sgio when starting domain and attaching disk qemu: Check if the shared disk's cdbfilter conflicts with others Pushed with the small nits pointed out by Eric fixed. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 10/11] target-i386: Call kvm_check_features_against_host() only if CONFIG_KVM is set
On Mon, 7 Jan 2013 15:30:26 +0200
Gleb Natapov g...@redhat.com wrote:
On Mon, Jan 07, 2013 at 02:15:14PM +0100, Igor Mammedov wrote:
On Mon, 7 Jan 2013 10:00:09 -0200
Eduardo Habkost ehabk...@redhat.com wrote:
On Sun, Jan 06, 2013 at 04:27:19PM +0200, Gleb Natapov wrote:
On Fri, Jan 04, 2013 at 08:01:11PM -0200, Eduardo Habkost wrote:
This will be necessary once kvm_check_features_against_host() starts
using KVM-specific definitions (so it won't compile anymore if
CONFIG_KVM is not set).
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1c3c7e1..876b0f6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -936,6 +936,7 @@ static void kvm_cpu_fill_host(x86_def_t
*x86_cpu_def)
#endif /* CONFIG_KVM */
}
+#ifdef CONFIG_KVM
static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
{
int i;
@@ -987,6 +988,7 @@ static int
kvm_check_features_against_host(x86_def_t *guest_def)
}
return rv;
}
+#endif
static void x86_cpuid_version_get_family(Object *obj, Visitor *v,
void *opaque,
const char *name, Error
**errp)
@@ -1410,10 +1412,12 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-kvm_features = ~minus_kvm_features;
x86_cpu_def-svm_features = ~minus_svm_features;
x86_cpu_def-cpuid_7_0_ebx_features = ~minus_7_0_ebx_features;
+#ifdef CONFIG_KVM
if (check_cpuid kvm_enabled()) {
if (kvm_check_features_against_host(x86_cpu_def)
enforce_cpuid)
goto error;
}
+#endif
Provide kvm_check_features_against_host() stub if !CONFIG_KVM and drop
ifdef here.
I will do. Igor probably will have to change his target-i386: move
kvm_check_features_against_host() check to realize time patch to use
the same approach, too.
Gleb,
Why do stub here? As result we will be adding more ifdef-s just in other
places. Currently kvm_cpu_fill_host(), unavailable_host_feature() and
Why will we be adding more ifdef-s in other places?
unavailable_host_feature() is being ifdef-ed above
kvm_check_features_against_host() are bundled together in cpu.c so we could
instead ifdef whole block. Like here:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg146536.html
That's fine, but you can avoid things like:
if (kvm_enabled() name strcmp(name, host) == 0) {
+#ifdef CONFIG_KVM
kvm_cpu_fill_host(x86_cpu_def);
+#endif
in your patch by providing stub for kvm_cpu_fill_host() for !CONFIG_KVM
case. This is common practice really. Avoid ifdefs in the code.
This ifdef could be eliminated later when cpus are converted into sub-classes.
Then we would put host subclass close to kvm_cpu_fill_host inside of the same
ifdef. that would leave ifdef around kvm_check_features_against_host() in
cpu_x86_parse_featurestr().
For me code looks more readable with ifdef here, if we have stub, a reader
would have to look at kvm_check_features_against_host() body to see if it
does
anything.
If reader cares about kvm it has to anyway. If he does not, there is
friendly kvm_enabled() (which is stub in case of !CONFIG_KVM BTW) to
tell him that he does not care. No need additional ifdef there.
both ways would work, but if stubs are preferred style then there is no
point arguing.
--
Gleb.
--
Regards,
Igor
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java 7/9] Use and implement virDomainMigrate2 instead of virDomainMigrate
At Sat, 5 Jan 2013 12:48:24 +0100,
Wido den Hollander wrote:
This enables use a different XML for running the guest on the target host.
This sentence does not sound right. How about
This makes it possible to alter host-specific portions of the domain
XML that will be used on the destination host.
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/Domain.java | 54
++--
src/main/java/org/libvirt/jna/Libvirt.java |2 ++
2 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index 932f56c..03afa0e 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -760,6 +760,8 @@ public class Domain {
*
* @param dconn
*destination host (a Connect object)
+ * @param dxml
+ *(optional) XML config for launching guest on target
* @param flags
*flags
* @param dname
@@ -773,13 +775,61 @@ public class Domain {
* scope of the destination connection (dconn).
* @throws LibvirtException
*/
-public Domain migrate(Connect dconn, long flags, String dname, String
uri, long bandwidth) throws LibvirtException {
-DomainPointer newPtr = libvirt.virDomainMigrate(VDP, dconn.VCP, new
NativeLong(flags), dname, uri, new NativeLong(bandwidth));
+public Domain migrate(Connect dconn, long flags, String dxml, String
dname, String uri, long bandwidth) throws LibvirtException {
+DomainPointer newPtr = libvirt.virDomainMigrate2(VDP, dconn.VCP,
dxml, new NativeLong(flags), dname, uri, new NativeLong(bandwidth));
processError();
return new Domain(dconn, newPtr);
}
/**
+ * Migrate this domain object from its current host to the destination
host
+ * given by dconn (a connection to the destination host). Flags may be
one
+ * of more of the following: Domain.VIR_MIGRATE_LIVE Attempt a live
+ * migration. If a hypervisor supports renaming domains during migration,
+ * then you may set the dname parameter to the new name (otherwise it
keeps
+ * the same name). If this is not supported by the hypervisor, dname
must be
+ * NULL or else you will get an error. Since typically the two
hypervisors
+ * connect directly to each other in order to perform the migration, you
may
+ * need to specify a path from the source to the destination. This is the
+ * purpose of the uri parameter.If uri is NULL, then libvirt will try to
+ * find the best method. Uri may specify the hostname or IP address of
the
+ * destination host as seen from the source, or uri may be a URI giving
+ * transport, hostname, user, port, etc. in the usual form. Uri should
only
+ * be specified if you want to migrate over a specific interface on the
+ * remote host. For Qemu/KVM, the uri should be of the form
+ * tcp://hostname[:port]. This does not require TCP auth to be setup
+ * between the connections, since migrate uses a straight TCP connection
+ * (unless using the PEER2PEER flag, in which case URI should be a full
+ * fledged libvirt URI). Refer also to driver documentation for the
+ * particular URIs supported. If set to 0, libvirt will choose a suitable
+ * default. Some hypervisors do not support this feature and will return
an
+ * error if bandwidth is not 0. To see which features are supported by
the
+ * current hypervisor, see Connect.getCapabilities,
+ * /capabilities/host/migration_features. There are many limitations on
+ * migration imposed by the underlying technology - for example it may
not
+ * be possible to migrate between different processors even with the same
+ * architecture, or between different types of hypervisor.
Breaking this up into a few paragraphs would be nice.
Also, you've re-used the documentation of the old method for the new
method having an additional argument. It would be nice to add some
words about the new parameter. And while touching this, adding a few
paragraph separators would be a bonus.
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java
b/src/main/java/org/libvirt/jna/Libvirt.java
index 2bbc8c3..de2a262 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -209,6 +209,8 @@ public interface Libvirt extends Library {
public int virDomainManagedSaveRemove(DomainPointer virDomainPtr, int
flags);
public DomainPointer virDomainMigrate(DomainPointer virDomainPtr,
ConnectionPointer virConnectPtr,
NativeLong flags, String dname, String uri, NativeLong
bandwidth);
+public DomainPointer virDomainMigrate2(DomainPointer virDomainPtr,
ConnectionPointer virConnectPtr,
+String dxml, NativeLong
Re: [libvirt] [PATCH 00/15] [libvirt-java] some improvements and bug fixes (resend
On Wed, Nov 28, 2012 at 10:51:56 +0100, Claudio Bley wrote: ping? Anyone care to review these patches, before someone redoes this work again...? ACK to the whole series. I looked at the individual patches and while I'm not a java nor jna expert, the patches look like they're doing the right thing :-) Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java 8/9] Implement and use virDomainMigrateToURI2 instead of virDomainMigrateToURI
At Sat, 5 Jan 2013 12:48:25 +0100,
Wido den Hollander wrote:
The migrateToURI method now uses virDomainMigrateToURI2 so we can support
some more features.
Signed-off-by: Wido den Hollander w...@widodh.nl
---
src/main/java/org/libvirt/Domain.java | 33
+---
src/main/java/org/libvirt/jna/Libvirt.java |2 ++
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index 03afa0e..6fb1161 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -855,6 +855,35 @@ public class Domain {
*
href=http://www.libvirt.org/html/libvirt-libvirt.html#virDomainMigrateToURI;
* virDomainMigrateToURI/a
*
+ * @param dconnuri
+ *(optional) URI for target libvirtd if @flags includes
VIR_MIGRATE_PEER2PEER
+ * @param miguri
+ *(optional) URI for invoking the migration, not if @flags
includs VIR_MIGRATE_TUNNELLED
+ * @param dxml
+ *(optional) XML config for launching guest on target
+ * @param flags
+ *Controls the migrate
+ * @param dname
+ *The name at the destnation
+ * @param bandwidth
+ *Specify the migration bandwidth
+ * @return 0 if successful, -1 if not
Actually no. It throws an exception in case of an error.
I think it would be awkward introducing a new method with a known
glitch now, just in order to fix it later.
+ * @throws LibvirtException
+ */
+public int migrateToURI(String dconnuri, String miguri, String dxml,
long flags, String dname, long bandwidth) throws LibvirtException {
+int returnValue = libvirt.virDomainMigrateToURI2(VDP, dconnuri,
miguri, dxml, new NativeLong(flags), dname, new NativeLong(bandwidth));
+processError();
+return returnValue;
+}
+
+/**
+ * Migrate the domain object from its current host to the destination
host
+ * given by duri.
+ *
+ * @see a
+ *
href=http://www.libvirt.org/html/libvirt-libvirt.html#virDomainMigrateToURI;
+ * virDomainMigrateToURI/a
+ *
* @param uri
*The destination URI
* @param flags
@@ -867,9 +896,7 @@ public class Domain {
* @throws LibvirtException
*/
public int migrateToURI(String uri, long flags, String dname, long
bandwidth) throws LibvirtException {
-int returnValue = libvirt.virDomainMigrateToURI(VDP, uri, new
NativeLong(flags), dname, new NativeLong(bandwidth));
-processError();
-return returnValue;
+return migrateToURI(uri, null, null, flags, dname, bandwidth);
}
/**
diff --git a/src/main/java/org/libvirt/jna/Libvirt.java
b/src/main/java/org/libvirt/jna/Libvirt.java
index de2a262..27011d9 100644
--- a/src/main/java/org/libvirt/jna/Libvirt.java
+++ b/src/main/java/org/libvirt/jna/Libvirt.java
@@ -214,6 +214,8 @@ public interface Libvirt extends Library {
public int virDomainMigrateSetMaxDowntime(DomainPointer virDomainPtr,
long downtime, int flags);
public int virDomainMigrateToURI(DomainPointer virDomainPtr, String duri,
NativeLong flags, String dname, NativeLong bandwidth);
+public int virDomainMigrateToURI2(DomainPointer virDomainPtr, String
dconnuri, String miguri,
+String dxml, NativeLong flags, String dname, NativeLong
bandwidth);
public int virDomainMemoryStats(DomainPointer virDomainPtr,
virDomainMemoryStats[] stats, int nr_stats, int flags);
public int virDomainPinVcpu(DomainPointer virDomainPtr, int vcpu, byte[]
cpumap, int maplen);
public int virDomainReboot(DomainPointer virDomainPtr, int flags);
You know the drill: please drop the public modifier ;-)
And just remove the virDomainMigrateToURI in favor of virDomainMigrateToURI2.
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] [PATCH 1/4] FreeBSD: implement virNetDevExists() and virNetDevSetName()
Not a regular reviewer (yet), but I figured I'd give this series a
look and provide some feedback...
On 01/04/2013 10:00 AM, Roman Bogorodskiy wrote:
---
src/util/virnetdev.c | 37 ++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 7ffd3c2..c7eeb50 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -83,10 +83,32 @@ static int virNetDevSetupControl(const char *ifname,
{
return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM);
}
-#endif
+#elif defined(__FreeBSD__)
+static int virNetDevSetupControl(const char *ifname,
+ struct ifreq *ifr)
+{
Why not use virNetDevSetupControlFull() passing AF_LOCAL, SOCK_DGRAM?
Does the virSetInherit() not apply for FreeBSD?
+int s;
+memset(ifr, 0, sizeof(*ifr));
-#if defined(SIOCGIFFLAGS) defined(HAVE_STRUCT_IFREQ)
+if (virStrcpyStatic(ifr-ifr_name, ifname) == NULL) {
+virReportSystemError(ERANGE,
+ _(Network interface name '%s' is too long),
+ ifname);
+return -1;
+}
+
+if ((s = socket(AF_LOCAL, SOCK_DGRAM, 0)) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot open network interface control
socket));
+return -1;
+}
+
+return s;
+}
+#endif
+
+#if defined(SIOCGIFFLAGS) (defined(HAVE_STRUCT_IFREQ) ||
defined(__FreeBSD__))
/**
* virNetDevExists:
* @ifname
@@ -105,7 +127,12 @@ int virNetDevExists(const char *ifname)
return -1;
if (ioctl(fd, SIOCGIFFLAGS, ifr)) {
+/* FreeBSD throws ENXIO when interface doesn't exist */
+#if defined(__FreeBSD__)
+if (errno == ENXIO)
+#else
if (errno == ENODEV)
+#endif
ret = 0;
else
virReportSystemError(errno,
@@ -459,7 +486,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t
pidInNs)
return rc;
}
-#if defined(SIOCSIFNAME) defined(HAVE_STRUCT_IFREQ)
+#if defined(SIOCSIFNAME) (defined(HAVE_STRUCT_IFREQ) ||
defined(__FreeBSD__))
/**
* virNetDevSetName:
* @ifname: name of device
@@ -478,12 +505,16 @@ int virNetDevSetName(const char* ifname, const char
*newifname)
if ((fd = virNetDevSetupControl(ifname, ifr)) 0)
return -1;
+#if defined(__FreeBSD__)
+ifr.ifr_data = newifname;
+#else
if (virStrcpyStatic(ifr.ifr_newname, newifname) == NULL) {
virReportSystemError(ERANGE,
_(Network interface name '%s' is too long),
newifname);
goto cleanup;
}
+#endif
if (ioctl(fd, SIOCSIFNAME, ifr)) {
virReportSystemError(errno,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add missing flags to migrate documentation
--- src/libvirt.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index 6d1da12..83aeb23 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5121,10 +5121,15 @@ virDomainMigrateDirect(virDomainPtr domain, * VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the * domain on the source host. * VIR_MIGRATE_PAUSEDLeave the domain suspended on the remote side. + * VIR_MIGRATE_NON_SHARED_DISK Migration with non-shared storage with full + * disk copy + * VIR_MIGRATE_NON_SHARED_INC Migration with non-shared storage with + * incremental disk copy * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. + * VIR_MIGRATE_OFFLINE Migrate offline * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5330,10 +5335,15 @@ error: * VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the * domain on the source host. * VIR_MIGRATE_PAUSEDLeave the domain suspended on the remote side. + * VIR_MIGRATE_NON_SHARED_DISK Migration with non-shared storage with full + * disk copy + * VIR_MIGRATE_NON_SHARED_INC Migration with non-shared storage with + * incremental disk copy * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. + * VIR_MIGRATE_OFFLINE Migrate offline * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5556,10 +5566,16 @@ error: *on the destination host. * VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the * domain on the source host. + * VIR_MIGRATE_PAUSEDLeave the domain suspended on the remote side. + * VIR_MIGRATE_NON_SHARED_DISK Migration with non-shared storage with full + * disk copy + * VIR_MIGRATE_NON_SHARED_INC Migration with non-shared storage with + * incremental disk copy * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. + * VIR_MIGRATE_OFFLINE Migrate offline * * The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag. * If the VIR_MIGRATE_PEER2PEER flag is NOT set, the duri parameter @@ -5689,10 +5705,16 @@ error: *on the destination host. * VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the * domain on the source host. + * VIR_MIGRATE_PAUSEDLeave the domain suspended on the remote side. + * VIR_MIGRATE_NON_SHARED_DISK Migration with non-shared storage with full + * disk copy + * VIR_MIGRATE_NON_SHARED_INC Migration with non-shared storage with + * incremental disk copy * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration * changes during the migration process (set * automatically when supported). * VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe. + * VIR_MIGRATE_OFFLINE Migrate offline * * The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag. * -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make TLS support conditional
From: Daniel P. Berrange berra...@redhat.com Add checks for existance of GNUTLS and automatically disable it if not found. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- configure.ac | 70 --- daemon/libvirtd.c | 41 ++--- daemon/remote.c | 2 ++ src/Makefile.am | 8 - src/libvirt.c | 17 --- src/locking/lock_daemon.c | 12 ++-- src/lxc/lxc_controller.c | 6 ++-- src/qemu/qemu_migration.c | 15 -- src/remote/remote_driver.c| 15 ++ src/rpc/virnetclient.c| 20 ++--- src/rpc/virnetclient.h| 8 - src/rpc/virnetserver.c| 6 src/rpc/virnetserver.h| 6 +++- src/rpc/virnetserverclient.c | 63 ++ src/rpc/virnetserverclient.h | 4 +++ src/rpc/virnetserverservice.c | 31 ++- src/rpc/virnetserverservice.h | 20 + src/rpc/virnetsocket.c| 17 ++- src/rpc/virnetsocket.h| 6 +++- tests/Makefile.am | 11 ++- 20 files changed, 311 insertions(+), 67 deletions(-) diff --git a/configure.ac b/configure.ac index ab08f17..bb64bf6 100644 --- a/configure.ac +++ b/configure.ac @@ -1025,30 +1025,62 @@ CFLAGS=$old_cflags LIBS=$old_libs dnl GnuTLS library -GNUTLS_CFLAGS= -GNUTLS_LIBS= -GNUTLS_FOUND=no -if test -x $PKG_CONFIG ; then - PKG_CHECK_MODULES(GNUTLS, gnutls = $GNUTLS_REQUIRED, - [GNUTLS_FOUND=yes], [GNUTLS_FOUND=no]) -fi -if test $GNUTLS_FOUND = no; then +AC_ARG_WITH([gnutls], + AC_HELP_STRING([--with-gnutls], [use GNUTLS for encryption @:@default=check@:@]), + [], + [with_gnutls=check]) + + +if test x$with_gnutls != xno; then + if test x$with_gnutls != xyes test x$with_gnutls != xcheck; then +GNUTLS_CFLAGS=-I$with_gnutls/include +GNUTLS_LIBS=-L$with_gnutls/lib + fi fail=0 + old_cflags=$CFLAGS old_libs=$LIBS - AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1]) - AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt]) + CFLAGS=$CFLAGS $GNUTLS_CFLAGS + LIBS=$LIBS $GNUTLS_LIBS - test $fail = 1 -AC_MSG_ERROR([You must install the GnuTLS library in order to compile and run libvirt]) + GNUTLS_FOUND=no + if test -x $PKG_CONFIG ; then +PKG_CHECK_MODULES(GNUTLS, gnutls = $GNUTLS_REQUIRED, + [GNUTLS_FOUND=yes], [GNUTLS_FOUND=no]) + fi + if test $GNUTLS_FOUND = no; then +fail=0 +AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1]) +AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt]) + +test $fail = 0 GNUTLS_FOUND=yes + +GNUTLS_LIBS=$GNUTLS_LIBS -lgnutls + fi + if test $GNUTLS_FOUND = no; then +if test $with_gnutls = check; then + with_gnutls=no + GNUTLS_LIBS= + GNUTLS_CFLAGS= +else + AC_MSG_ERROR([You must install the GnuTLS library in order to compile and run libvirt]) +fi + else +dnl Not all versions of gnutls include -lgcrypt, and so we add +dnl it explicitly for the calls to gcry_control/check_version +GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt + +with_gnutls=yes + fi - dnl Not all versions of gnutls include -lgcrypt, and so we add - dnl it explicitly for the calls to gcry_control/check_version - GNUTLS_LIBS=$LIBS -lgcrypt LIBS=$old_libs -else - GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt + CFLAGS=$old_CFLAGS fi +if test x$with_gnutls = xyes ; then + AC_DEFINE_UNQUOTED([HAVE_GNUTLS], 1, + [whether GNUTLS is available for encryption]) +fi +AM_CONDITIONAL([HAVE_GNUTLS], [test x$with_gnutls = xyes]) AC_SUBST([GNUTLS_CFLAGS]) AC_SUBST([GNUTLS_LIBS]) @@ -3168,7 +3200,11 @@ AC_MSG_NOTICE([ libssh2: $LIBSSH2_CFLAGS $LIBSSH2_LIBS]) else AC_MSG_NOTICE([ libssh2: no]) fi +if test $with_gnutls != no ; then AC_MSG_NOTICE([ gnutls: $GNUTLS_CFLAGS $GNUTLS_LIBS]) +else +AC_MSG_NOTICE([ gnutls: no]) +fi if test $with_sasl != no ; then AC_MSG_NOTICE([sasl: $SASL_CFLAGS $SASL_LIBS]) else diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index fa4d129..ff54af3 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -449,7 +449,9 @@ static int daemonSetupNetworking(virNetServerPtr srv, virNetServerServicePtr svc = NULL; virNetServerServicePtr svcRO = NULL; virNetServerServicePtr svcTCP = NULL; +#if HAVE_GNUTLS virNetServerServicePtr svcTLS = NULL; +#endif gid_t unix_sock_gid = 0; int unix_sock_ro_mask = 0; int unix_sock_rw_mask = 0; @@ -474,9 +476,11 @@ static int daemonSetupNetworking(virNetServerPtr srv, unix_sock_rw_mask, unix_sock_gid, config-auth_unix_rw, +#if HAVE_GNUTLS + NULL, +#endif false, - config-max_client_requests, -
Re: [libvirt] [PATCH 2/4] FreeBSD: implement virNetDevTapCreate() and virNetDevTapDelete().
On 01/04/2013 10:00 AM, Roman Bogorodskiy wrote:
---
src/util/virnetdevtap.c | 140
++--
1 file changed, 137 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index a884de1..426d629 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -103,7 +103,7 @@ virNetDevProbeVnetHdr(int tapfd)
#endif
-#ifdef TUNSETIFF
+#if defined(TUNSETIFF)
/**
* virNetDevTapCreate:
* @ifname: the interface name
@@ -230,7 +230,141 @@ cleanup:
VIR_FORCE_CLOSE(fd);
return ret;
}
-#else /* ! TUNSETIFF */
Change the comment to add the !defined(TUNSETIFF)
+#elif defined(__FreeBSD__)
+int virNetDevTapCreate(char **ifname,
+ int *tapfd,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
+int s;
+struct ifreq ifr;
+int ret = -1;
+
+s = socket(AF_LOCAL, SOCK_DGRAM, 0);
+if (s 0) {
+virReportSystemError(errno, %s,
+ _(Unable to create socket));
+return -1;
+}
+
+memset(ifr, 0, sizeof(ifr));
+
+if (virStrcpyStatic(ifr.ifr_name, tap) == NULL) {
+virReportSystemError(ERANGE,
+ _(Network interface name '%s' is too long),
+ *ifname);
+goto cleanup;
+
+}
Confused here - you are copying tap, but using *ifname in the error
message. Why not use the virNetDevSetupControl() if in fact you are
trying to use *ifname?
+
+if (ioctl(s, SIOCIFCREATE2, ifr) 0) {
+virReportSystemError(errno,
+ _(Unable to create tap device %s),
+ NULLSTR(*ifname));
+goto cleanup;
+}
+
+char *newifname = NULL;
Put this with the other declarations.
+if (strstr(*ifname, %d) == NULL) {
+ newifname = *ifname;
+} else {
+int i;
+for (i = 0; i = 255; i++) {
+virBuffer newname = VIR_BUFFER_INITIALIZER;
+virBufferAsprintf(newname, *ifname, i);
+
+if (virBufferError(newname)) {
+virBufferFreeAndReset(newname);
+virReportOOMError();
+ret = - 1;
Redundant with initialization
+goto cleanup;
+}
+
+if (virNetDevExists(virBufferCurrentContent(newname)) == 0) {
+newifname = strdup(virBufferContentAndReset(newname));
+break;
+}
In this instance memory is allocated for newifname, when is it free()'d?
Secondarily if not allocated, then the general message below may mask
the reason. That is although you failed to generate a new name, it was
because you were out of memory.
+}
+}
+
+if (newifname == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to generate new name for interface %s),
+ ifr.ifr_name);
+ret = -1;
Redundant with initialization
+goto cleanup;
+}
+
+if (virNetDevSetName(ifr.ifr_name, newifname) == -1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to rename interface %s to %s),
+ ifr.ifr_name, newifname);
This message is redundant with the virNetDevSetName() function.
+ret = -1;
Redundant with initialization
+goto cleanup;
+}
+
+*ifname = newifname;
This is where a possible memory leak occurs if *ifname had been alloc'd
before we'd be now losing it. I see the other virNetDevTapCreate() will
VIR_FREE(*ifname) prior to strdup() attempt.
Secondarily, it seems to me that if we go through the if strstr() ==
NULL path, then we're not really changing the name, correct? If that's
the case, then the only time you need to change the name is in the else
condition.
I think the answer probably goes back to the virStrcpyStatic using tap
above for what the expectations are here.
+
+if (tapfd) {
+char *dev_prefix = /dev/;
+char *dev_path;
+int dev_path_len = strlen(dev_prefix) + strlen(*ifname) + 1;
+
+if (VIR_ALLOC_N(dev_path, dev_path_len) 0) {
+virReportOOMError();
+ret = -1;
Redundant with initialization
+goto cleanup;
+}
+snprintf(dev_path, dev_path_len, %s%s, dev_prefix, *ifname);
+
+*tapfd = open(dev_path, O_RDWR);
+
+VIR_FREE(dev_path);
+}
+
+ret = 0;
+cleanup:
+if (ret 0)
+VIR_FORCE_CLOSE(s);
+
+return ret;
+}
+
+int virNetDevTapDelete(const char *ifname)
+{
+int s;
+struct ifreq ifr;
+int ret = -1;
+
+s = socket(AF_LOCAL, SOCK_DGRAM, 0);
+if (s 0) {
+virReportSystemError(errno, %s,
+ _(Unable to create socket));
+return -1;
+}
+
+memset(ifr, 0, sizeof(ifr));
+
Re: [libvirt] [PATCH 4/4] FreeBSD: implement virNetDevGetMTU().
On 01/04/2013 10:00 AM, Roman Bogorodskiy wrote:
---
src/util/virnetdev.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 6e4f7ad..809c0f7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -427,6 +427,32 @@ cleanup:
VIR_FORCE_CLOSE(fd);
return ret;
}
+#elif defined(__FreeBSD__)
+int virNetDevGetMTU(const char *ifname)
+{
+int s;
+int ret;
+struct ifreq ifr;
+
+if ((s = virNetDevSetupControl(ifname, ifr)) 0)
+return -1;
+
+ifr.ifr_addr.sa_family = AF_INET;
+virStrcpyStatic(ifr.ifr_name, ifname);
Ignoring the result of the virStrcpyStatic() call.
+if (ioctl(s, SIOCGIFMTU, (caddr_t)ifr) 0) {
+virReportSystemError(errno,
+ _(Cannot get interface MTU on '%s'),
+ifname);
+ret = -1;
+goto cleanup;
+}
+
+ret = ifr.ifr_mtu;
+
+cleanup:
+VIR_FORCE_CLOSE(s);
+return ret;
+}
#else
int virNetDevGetMTU(const char *ifname)
{
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] FreeBSD: implement virNetDevSetMAC().
On 01/04/2013 10:00 AM, Roman Bogorodskiy wrote:
---
src/util/virnetdev.c | 50 ++
1 file changed, 50 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index c7eeb50..6e4f7ad 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -42,6 +42,11 @@
# undef HAVE_STRUCT_IFREQ
#endif
+#if defined(__FreeBSD__)
+# include sys/sockio.h
+# include net/if_dl.h
+#endif
+
#define VIR_FROM_THIS VIR_FROM_NONE
#if defined(HAVE_STRUCT_IFREQ)
@@ -200,6 +205,51 @@ cleanup:
VIR_FORCE_CLOSE(fd);
return ret;
}
+#elif defined(__FreeBSD__)
+int virNetDevSetMAC(const char *ifname,
+const virMacAddrPtr macaddr)
+{
+struct ifreq ifr;
+struct sockaddr_dl sdl;
+uint8_t mac[19];
19 was odd to me... I found VIR_MAC_STRING_BUFLEN (which is essentially
18) using cscope. Figured it would be better to go that route - it does
make it easier to find code later on that's adjust the MAC
+char *macstr;
+int s;
+int ret = -1;
+
+if ((s = virNetDevSetupControl(ifname, ifr)) 0)
+return -1;
+
+if (VIR_ALLOC_N(macstr, VIR_MAC_STRING_BUFLEN) 0) {
+virReportOOMError();
+ret = - 1;
Redundant with the initialization
+goto cleanup;
+}
+virMacAddrFormat(macaddr, macstr);
+
+virStrncpy(ifr.ifr_name, ifname, IFNAMSIZ, sizeof(ifr.ifr_name));
Ignoring the status of the virStrncpy()
+memset(mac, 0, sizeof(mac));
+mac[0] = ':';
+virStrncpy(mac + 1, macstr, strlen(macstr), sizeof(mac));
Ignoring the status of the virStrncpy()
+sdl.sdl_len = sizeof(sdl);
+link_addr(mac, sdl);
+
+bcopy(sdl.sdl_data, ifr.ifr_addr.sa_data, 6);
+ifr.ifr_addr.sa_len = 6;
+
+if (ioctl(s, SIOCSIFLLADDR, ifr) 0) {
+virReportSystemError(errno,
+ _(Cannot set interface MAC on '%s'),
+ifname);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(macstr);
+VIR_FORCE_CLOSE(s);
+
+return ret;
+}
#else
int virNetDevSetMAC(const char *ifname,
const virMacAddrPtr macaddr ATTRIBUTE_UNUSED)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt-java 9/9] Add resize flags to StorageVol
On Mon, Jan 07, 2013 at 12:38:16 +0100, Claudio Bley wrote: ... Additionally, consensus was that we should drop the C prefixes from enum values when wrapping them in Java. I will do this for existing enums when I find the time, but for new enums we could just as well avoid introducing them in the first place. In this case the prefix of VIR_DOMAIN_BLOCK_RESIZE should be removed. Usage would thus be Domain.BlockResizeFlags.BYTES This is certainly good for new enums, however, you need to be careful when changing existing enums. You would need to keep the long names for backward compatibility. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] build: install libvirt sysctl file correctly
On 01/05/2013 11:27 AM, Jim Fehlig wrote: Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=887017 reports that even though libvirt attempts to set fs.aio-max-nr via sysctl, the file was installed with the wrong name and gets ignored by sysctl. Furthermore, 'man systcl.d' recommends that packages install into hard-coded /usr/lib/sysctl.d (even when libdir is /usr/lib64), so that sysadmins can use /etc/sysctl.d for overrides. * daemon/Makefile.am (install-sysctl, uninstall-sysctl): Use correct location. * libvirt.spec.in (network_files): Reflect this. --- daemon/Makefile.am | 10 ++ libvirt.spec.in| 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) I didn't test this one in my rpm builds, but looks good to me. ACK. Thanks for the reviews; I've gone ahead and pushed the series now (trusting that my testing via autobuild.sh is sufficient for this one). I am also in the process of backporting these to v0.10.2-maint, since that is the version used by F18 where the bug report was first raised. -- 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 libvirt-java 9/9] Add resize flags to StorageVol
At Mon, 7 Jan 2013 16:41:14 +0100, Jiri Denemark wrote: On Mon, Jan 07, 2013 at 12:38:16 +0100, Claudio Bley wrote: ... Additionally, consensus was that we should drop the C prefixes from enum values when wrapping them in Java. I will do this for existing enums when I find the time, but for new enums we could just as well avoid introducing them in the first place. In this case the prefix of VIR_DOMAIN_BLOCK_RESIZE should be removed. Usage would thus be Domain.BlockResizeFlags.BYTES This is certainly good for new enums, however, you need to be careful when changing existing enums. You would need to keep the long names for backward compatibility. This is certainly true. But there are some issues that require changing the API without being able to provide backward compatibility. (Changing method return types, without changing their parameter list, so overloading does not help here). The plan is to drop backward compatibility at some point and fix all of those issues. Probably in libvirt-java 0.5.x. 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
[libvirt] [PATCH] Speed up fallback to legacy non-QMP probing
From: Daniel P. Berrange berra...@redhat.com
Since we daemonized QEMU for capabilities probing there is a long
time if QEMU fals to launch. This is because we're not passing in
any virDomainObjPtr instance and thus the monitor code can not
check to see if the PID is still alive.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_capabilities.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e16bc70..bcecc6d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2307,6 +2307,8 @@ qemuCapsInitQMP(qemuCapsPtr caps,
char *pidfile = NULL;
qemuCapsHookData hookData;
char *archstr;
+pid_t pid = 0;
+virDomainObj vm;
/* the .sock sufix is important to avoid a possible clash with a qemu
* domain called capabilities
@@ -2360,7 +2362,16 @@ qemuCapsInitQMP(qemuCapsPtr caps,
goto cleanup;
}
-if (!(mon = qemuMonitorOpen(NULL, config, true, callbacks))) {
+if (virPidFileReadPath(pidfile, pid) 0) {
+VIR_DEBUG(Failed to read pidfile %s, pidfile);
+ret = 0;
+goto cleanup;
+}
+
+memset(vm, 0, sizeof(vm));
+vm.pid = pid;
+
+if (!(mon = qemuMonitorOpen(vm, config, true, callbacks))) {
ret = 0;
goto cleanup;
}
@@ -2446,21 +2457,16 @@ cleanup:
VIR_FREE(monpath);
VIR_FREE(package);
-if (pidfile) {
+if (pid != 0) {
char ebuf[1024];
-pid_t pid;
-int rc;
-if ((rc = virPidFileReadPath(pidfile, pid)) 0) {
-VIR_DEBUG(Failed to read pidfile %s: %s,
- pidfile, virStrerror(-rc, ebuf, sizeof(ebuf)));
-} else {
-VIR_DEBUG(Killing QMP caps process %lld, (long long) pid);
-if (virProcessKill(pid, SIGKILL) 0 errno != ESRCH)
-VIR_ERROR(_(Failed to kill process %lld: %s),
- (long long) pid,
- virStrerror(errno, ebuf, sizeof(ebuf)));
-}
+VIR_DEBUG(Killing QMP caps process %lld, (long long) pid);
+if (virProcessKill(pid, SIGKILL) 0 errno != ESRCH)
+VIR_ERROR(_(Failed to kill process %lld: %s),
+ (long long) pid,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+}
+if (pidfile) {
unlink(pidfile);
VIR_FREE(pidfile);
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Only initialize capabilities after setting dir permissions
From: Daniel P. Berrange berra...@redhat.com The current code is initializing capabilities before setting directory permissions. Thus the QEMU binaries being run may not have the ability to create the UNIX monitor socket on the first run of libvirtd. --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac4c094..cea76af 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -831,9 +831,6 @@ qemuStartup(bool privileged, if (!qemu_driver-capsCache) goto error; -if ((qemu_driver-caps = qemuCreateCapabilities(qemu_driver)) == NULL) -goto error; - if ((qemu_driver-activePciHostdevs = pciDeviceListNew()) == NULL) goto error; @@ -870,6 +867,9 @@ qemuStartup(bool privileged, } } +if ((qemu_driver-caps = qemuCreateCapabilities(qemu_driver)) == NULL) +goto error; + /* If hugetlbfs is present, then we need to create a sub-directory within * it, since we can't assume the root mount point has permissions that * will let our spawned QEMU instances use it. -- 1.8.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu
On 01/04/13 03:28, li guang wrote:
在 2013-01-03四的 16:13 +0100，Ján Tomko写道：
On 12/26/12 02:00, liguang wrote:
@@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
* When QEMU grows support for 1 PCI domain, then pci.0 change
* to pciNN.0 where NN is the domain number
*/
-if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS))
+if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) {
+virBufferAsprintf(buf, ,bus=pci-bridge%d,
info-addr.pci.bus);
+} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) {
virBufferAsprintf(buf, ,bus=pci.0);
Is there any way (or plan) to use more pci buses with QEMU other than
with the pci bridges? If not, we could just name the bridges pci.%d. (If
we index the bridges from 1).
as far as I know, qemu can't use multi-pci-bus,
so only pci.0 accepted now.
-else
+} else {
virBufferAsprintf(buf, ,bus=pci);
+}
if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
virBufferAddLit(buf, ,multifunction=on);
else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF)
@@ -3455,6 +3460,32 @@ error:
return NULL;
}
+char *
+qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev,
+ qemuCapsPtr caps, int idx)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(buf, pci-bridge,chassis_nr=1);
The chassis number has to be unique for each bridge.
chassis number is not so important,
here, I just set all bridges in same chassis.
Each bridge must have a unique chassis #.
You *must* set it to a value 0.
I don't check that it is unique, just that it is 0.
If you make it non unique, guest will be confused.
http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02765.html
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu
On Thu, Jan 03, 2013 at 04:13:52PM +0100, Ján Tomko wrote:
On 12/26/12 02:00, liguang wrote:
@@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
* When QEMU grows support for 1 PCI domain, then pci.0 change
* to pciNN.0 where NN is the domain number
*/
-if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS))
+if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) {
+virBufferAsprintf(buf, ,bus=pci-bridge%d,
info-addr.pci.bus);
+} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) {
virBufferAsprintf(buf, ,bus=pci.0);
Is there any way (or plan) to use more pci buses with QEMU other than
with the pci bridges? If not, we could just name the bridges pci.%d. (If
we index the bridges from 1).
That depends on the machine type you are using. The PC machine type only
has a single PCI bridge. IIRC, the q35 machine type has multiple bridges.
Either way I don't think this is a problem for libvirt. When we add PCI
bridge devices we should just use the same 'pci.%d' as Jan suggests.
If the machine type in question has 2 bridges by default, this simply
means the user adding bridges must start from bus=2.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] virsh: Use virNodeDeviceLookupByWWN
Only nodedev-destroy and nodedev-dumpxml can benifit from the
new API, other commands like nodedev-detach only works for
PCI devices, WWN makes no sense for them.
---
tools/virsh-nodedev.c | 91
tools/virsh.pod | 15 +---
2 files changed, 77 insertions(+), 29 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index f9517cc..70e05fab 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -97,8 +97,9 @@ static const vshCmdInfo info_node_device_destroy[] = {
};
static const vshCmdOptDef opts_node_device_destroy[] = {
-{name, VSH_OT_DATA, VSH_OFLAG_REQ,
- N_(name of the device to be destroyed)},
+{name, VSH_OT_ALIAS, 0, device},
+{device, VSH_OT_DATA, VSH_OFLAG_REQ,
+ N_(device name or wwn pair in 'wwnn,wwpn' format)},
{NULL, 0, 0, NULL}
};
@@ -106,21 +107,44 @@ static bool
cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
{
virNodeDevicePtr dev = NULL;
-bool ret = true;
-const char *name = NULL;
+bool ret = false;
+const char *device_value = NULL;
+char **arr = NULL;
+int narr;
-if (vshCommandOptString(cmd, name, name) = 0)
+if (vshCommandOptString(cmd, device, device_value) = 0)
return false;
-dev = virNodeDeviceLookupByName(ctl-conn, name);
+if (strchr(device_value, ',')) {
+narr = vshStringToArray(device_value, arr);
+if (narr != 2) {
+vshError(ctl, _(Malformed device value '%s'), device_value);
+goto cleanup;
+}
+
+dev = virNodeDeviceLookupByWWN(ctl-conn, arr[0], arr[1], 0);
+} else {
+dev = virNodeDeviceLookupByName(ctl-conn, device_value);
+}
+
+if (!dev) {
+vshError(ctl, %s '%s', _(Could not find matching device),
device_value);
+goto cleanup;
+}
if (virNodeDeviceDestroy(dev) == 0) {
-vshPrint(ctl, _(Destroyed node device '%s'\n), name);
+vshPrint(ctl, _(Destroyed node device '%s'\n), device_value);
} else {
-vshError(ctl, _(Failed to destroy node device '%s'), name);
-ret = false;
+vshError(ctl, _(Failed to destroy node device '%s'), device_value);
+goto cleanup;
}
+ret = true;
+cleanup:
+if (arr) {
+VIR_FREE(*arr);
+VIR_FREE(arr);
+}
virNodeDeviceFree(dev);
return ret;
}
@@ -459,34 +483,55 @@ static const vshCmdInfo info_node_device_dumpxml[] = {
static const vshCmdOptDef opts_node_device_dumpxml[] = {
-{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(device key)},
+{device, VSH_OT_DATA, VSH_OFLAG_REQ,
+ N_(device name or wwn pair in 'wwnn,wwpn' format)},
{NULL, 0, 0, NULL}
};
static bool
cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
{
-const char *name = NULL;
-virNodeDevicePtr device;
-char *xml;
-
-if (vshCommandOptString(cmd, device, name) = 0)
-return false;
-if (!(device = virNodeDeviceLookupByName(ctl-conn, name))) {
-vshError(ctl, %s '%s', _(Could not find matching device), name);
+virNodeDevicePtr device = NULL;
+char *xml = NULL;
+const char *device_value = NULL;
+char **arr = NULL;
+int narr;
+bool ret = false;
+
+if (vshCommandOptString(cmd, device, device_value) = 0)
return false;
+
+if (strchr(device_value, ',')) {
+narr = vshStringToArray(device_value, arr);
+if (narr != 2) {
+vshError(ctl, _(Malformed device value '%s'), device_value);
+goto cleanup;
+}
+
+device = virNodeDeviceLookupByWWN(ctl-conn, arr[0], arr[1], 0);
+} else {
+device = virNodeDeviceLookupByName(ctl-conn, device_value);
}
-xml = virNodeDeviceGetXMLDesc(device, 0);
-if (!xml) {
-virNodeDeviceFree(device);
-return false;
+if (!device) {
+vshError(ctl, %s '%s', _(Could not find matching device),
device_value);
+goto cleanup;
}
+if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
+goto cleanup;
+
vshPrint(ctl, %s\n, xml);
+ret = true;
+
+cleanup:
+if (arr) {
+VIR_FREE(*arr);
+VIR_FREE(arr);
+}
VIR_FREE(xml);
virNodeDeviceFree(device);
-return true;
+return ret;
}
/*
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 3687a4d..a2da9ef 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1950,11 +1950,12 @@ host nodes are available for use, but this allows
registration of
host hardware that libvirt did not automatically detect. Ifile
contains xml for a top-level device description of a node device.
-=item Bnodedev-destroy Inodedev
+=item Bnodedev-destroy Idevice
-Destroy (stop) a device on the host. Note that this makes libvirt
-quit managing a host device, and may even make that device unusable
-by the rest of the physical host until a reboot.
+Destroy (stop) a device on the host. Idevice can be either device
+name or wwn
[libvirt] [PATCH 09/12] nodedev: Refactor the helpers
This adds two util functions (virIsCapableFCHost and virIsCapableVport),
and rename helper check_fc_host_linux as detect_scsi_host_caps,
check_capable_vport_linux is removed, as it's abstracted to the util
function virIsCapableVport. detect_scsi_host_caps nows detect both
the fc_host and vport_ops capabilities. stat(2) is replaced with
access(2) for saving.
* src/util/virutil.h:
- Declare virIsCapableFCHost and virIsCapableVport
* src/util/virutil.c:
- Implement virIsCapableFCHost and virIsCapableVport
* src/node_device/node_device_linux_sysfs.c:
- Remove check_capable_vport_linux
- Rename check_fc_host_linux as detect_scsi_host_caps, and refactor
it a bit to detect both fc_host and vport_os capabilities
* src/node_device/node_device_driver.h:
- Change/remove the related declarations
* src/node_device/node_device_udev.c: (Use detect_scsi_host_caps)
* src/node_device/node_device_hal.c: (Likewise)
* src/node_device/node_device_driver.c (Likewise)
---
src/libvirt_private.syms |2 +
src/node_device/node_device_driver.c |7 +-
src/node_device/node_device_driver.h | 10 +--
src/node_device/node_device_hal.c |4 +-
src/node_device/node_device_linux_sysfs.c | 135 -
src/node_device/node_device_udev.c|3 +-
src/util/virutil.c| 73
src/util/virutil.h|3 +
8 files changed, 123 insertions(+), 114 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2e83747..41c0d84 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1293,6 +1293,8 @@ virGetUserName;
virGetUserRuntimeDirectory;
virHexToBin;
virIndexToDiskName;
+virIsCapableFCHost;
+virIsCapableVport;
virIsDevMapperDevice;
virParseNumber;
virParseVersionString;
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index ef85af5..db2f922 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -48,7 +48,7 @@ static int update_caps(virNodeDeviceObjPtr dev)
while (cap) {
/* The only caps that currently need updating are FC related. */
if (cap-type == VIR_NODE_DEV_CAP_SCSI_HOST) {
-check_fc_host(dev-def-caps-data);
+detect_scsi_host_caps(dev-def-caps-data);
}
cap = cap-next;
}
@@ -241,18 +241,15 @@ nodeDeviceLookupByWWN(virConnectPtr conn,
nodeDeviceLock(driver);
for (i = 0; i devs-count; i++) {
-
obj = devs-objs[i];
virNodeDeviceObjLock(obj);
cap = obj-def-caps;
while (cap) {
-
if (cap-type == VIR_NODE_DEV_CAP_SCSI_HOST) {
-check_fc_host(cap-data);
+detect_scsi_host_caps(cap-data);
if (cap-data.scsi_host.flags
VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
-
if (STREQ(cap-data.scsi_host.wwnn, wwnn)
STREQ(cap-data.scsi_host.wwpn, wwpn)) {
dev = virGetNodeDevice(conn, obj-def-name);
diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
index edd2915..17bd020 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -53,16 +53,12 @@ int nodedevRegister(void);
# ifdef __linux__
-# define check_fc_host(d) check_fc_host_linux(d)
-int check_fc_host_linux(union _virNodeDevCapData *d);
-
-# define check_vport_capable(d) check_vport_capable_linux(d)
-int check_vport_capable_linux(union _virNodeDevCapData *d);
+# define detect_scsi_host_caps(d) detect_scsi_host_caps_linux(d)
+int detect_scsi_host_caps_linux(union _virNodeDevCapData *d);
# else /* __linux__ */
-# define check_fc_host(d) (-1)
-# define check_vport_capable(d)(-1)
+# define detect_scsi_host_caps(d) (-1)
# endif /* __linux__ */
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 33e0170..18cdab8 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -234,14 +234,12 @@ static int gather_scsi_host_cap(LibHalContext *ctx, const
char *udi,
(void)get_int_prop(ctx, udi, scsi_host.host, (int *)d-scsi_host.host);
-retval = check_fc_host(d);
+retval = detect_scsi_host_caps(d);
if (retval == -1) {
goto out;
}
-retval = check_vport_capable(d);
-
out:
return retval;
}
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index 742a0bc..054afea 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -1,8 +1,8 @@
/*
- * node_device_hal_linuc.c: Linux specific code to gather device data
+ * node_device_linux_sysfs.c: Linux specific code to gather device data
* not available through HAL.
*
- *
[libvirt] [PATCH 10/12] nodedev: Dump max vports and vports in use for HBA's XML
This enrichs HBA's xml by dumping the number of max vports and
vports in use. Format is like:
capability type='vport_ops'
max_vports164/max_vports
vports5/vports
/capability
* docs/formatnode.html.in: (Document the new XML)
* docs/schemas/nodedev.rng: (Add the schema)
* src/conf/node_device_conf.h: (New member for data.scsi_host)
* src/node_device/node_device_linux_sysfs.c: (Collect the value of
max_vports and vports)
---
docs/formatnode.html.in | 10 --
docs/schemas/nodedev.rng |6 +++
src/conf/node_device_conf.c |7 +++-
src/conf/node_device_conf.h |2 +
src/node_device/node_device_linux_sysfs.c | 48 ++--
5 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index fcf..5712bcf 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -136,9 +136,13 @@
ddThe SCSI host number./dd
dtcodecapability/code/dt
ddCurrent capabilities include vports_ops (indicates
-vport operations are supported) and fc_host, the later
-implies following sub-elements: codewwnn/code,
-codewwpn/code, codefabric_wwn/code.
+vport operations are supported) and fc_host. vport_ops
+could contain two optional sub-elements: codevports/code,
+and codemax_vports/code. codevports/code shows the
+number of vport in use. codemax_vports/code shows the
+maximum vports the HBA supports. fc_host implies following
+sub-elements: codewwnn/code, codewwpn/code, and
+codefabric_wwn/code.
/dd
/dl
/dd
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 7c85815..b94cce6 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -267,6 +267,12 @@
attribute name='type'
valuevports_ops/value
/attribute
+element name='max_vports'
+ ref name='unsignedInt'/
+/element
+element name='vports'
+ ref name='unsignedInt'/
+/element
/define
define name='capscsihost'
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 819e6af..5962d58 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -392,7 +392,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
virBufferAddLit(buf, /capability\n);
}
if (data-scsi_host.flags VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) {
-virBufferAddLit(buf, capability type='vport_ops' /\n);
+virBufferAddLit(buf, capability type='vport_ops'\n);
+virBufferAsprintf(buf, max_vports%d/max_vports\n,
+ data-scsi_host.max_vports);
+virBufferAsprintf(buf, vports%d/vports\n,
+ data-scsi_host.vports);
+virBufferAddLit(buf, /capability\n);
}
break;
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 145d699..4e584a3 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -137,6 +137,8 @@ struct _virNodeDevCapsDef {
char *wwpn;
char *fabric_wwn;
unsigned int flags;
+int max_vports;
+int vports;
} scsi_host;
struct {
char *name;
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index 054afea..b498726 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -40,6 +40,8 @@
int
detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
{
+char *max_vports = NULL;
+char *vports = NULL;
int ret = -1;
VIR_DEBUG(Checking if host%d is an FC HBA, d-scsi_host.host);
@@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
d-scsi_host.host,
port_name,
- d-scsi_host.wwpn) == -1) {
+ d-scsi_host.wwpn) 0) {
VIR_ERROR(_(Failed to read WWPN for host%d), d-scsi_host.host);
goto cleanup;
}
@@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
d-scsi_host.host,
node_name,
- d-scsi_host.wwnn) == -1) {
+ d-scsi_host.wwnn) 0) {
VIR_ERROR(_(Failed to read WWNN for host%d), d-scsi_host.host);
goto cleanup;
}
@@ -66,23 +68,61 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
[libvirt] [PATCH 12/12] nodedev: Abstract nodeDeviceVportCreateDelete as util function
This abstracts nodeDeviceVportCreateDelete as an util function
virManageVport, which can be further used by later storage patches
(to support persistent vHBA, I don't want to create the vHBA
using the public API, that's awkful).
---
src/libvirt_private.syms |1 +
src/node_device/node_device_driver.c | 101 +++---
src/node_device/node_device_driver.h |5 --
src/util/virutil.c | 84
src/util/virutil.h | 11
5 files changed, 104 insertions(+), 98 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 41c0d84..90e16f3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1296,6 +1296,7 @@ virIndexToDiskName;
virIsCapableFCHost;
virIsCapableVport;
virIsDevMapperDevice;
+virManageVport;
virParseNumber;
virParseVersionString;
virPipeReadUntilEOF;
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index db2f922..75b07c5 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -408,91 +408,6 @@ cleanup:
return ret;
}
-
-static int
-nodeDeviceVportCreateDelete(const int parent_host,
-const char *wwpn,
-const char *wwnn,
-int operation)
-{
-int retval = 0;
-char *operation_path = NULL, *vport_name = NULL;
-const char *operation_file = NULL;
-
-switch (operation) {
-case VPORT_CREATE:
-operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX;
-break;
-case VPORT_DELETE:
-operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX;
-break;
-default:
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Invalid vport operation (%d)), operation);
-retval = -1;
-goto cleanup;
-break;
-}
-
-if (virAsprintf(operation_path,
-%shost%d%s,
-LINUX_SYSFS_FC_HOST_PREFIX,
-parent_host,
-operation_file) 0) {
-
-virReportOOMError();
-retval = -1;
-goto cleanup;
-}
-
-if (!virFileExists(operation_path)) {
-VIR_FREE(operation_path);
-if (virAsprintf(operation_path,
-%shost%d%s,
-LINUX_SYSFS_SCSI_HOST_PREFIX,
-parent_host,
-operation_file) 0) {
-virReportOOMError();
-retval = -1;
-goto cleanup;
-}
-
-if (!virFileExists(operation_path)) {
-VIR_ERROR(_(No vport operation path found for host%d),
- parent_host);
-retval = -1;
-goto cleanup;
-}
-}
-
-VIR_DEBUG(Vport operation path is '%s', operation_path);
-
-if (virAsprintf(vport_name,
-%s:%s,
-wwpn,
-wwnn) 0) {
-
-virReportOOMError();
-retval = -1;
-goto cleanup;
-}
-
-if (virFileWriteStr(operation_path, vport_name, 0) == -1) {
-virReportSystemError(errno,
- _(Write of '%s' to '%s' during
- vport create/delete failed),
- vport_name, operation_path);
-retval = -1;
-}
-
-cleanup:
-VIR_FREE(vport_name);
-VIR_FREE(operation_path);
-VIR_DEBUG(%s, _(Vport operation complete));
-return retval;
-}
-
-
static int
get_time(time_t *t)
{
@@ -594,10 +509,10 @@ nodeDeviceCreateXML(virConnectPtr conn,
goto cleanup;
}
-if (nodeDeviceVportCreateDelete(parent_host,
-wwpn,
-wwnn,
-VPORT_CREATE) == -1) {
+if (virManageVport(parent_host,
+ wwpn,
+ wwnn,
+ VPORT_CREATE) == -1) {
goto cleanup;
}
@@ -661,10 +576,10 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
goto out;
}
-if (nodeDeviceVportCreateDelete(parent_host,
-wwpn,
-wwnn,
-VPORT_DELETE) == -1) {
+if (virManageVport(parent_host,
+ wwpn,
+ wwnn,
+ VPORT_DELETE) == -1) {
goto out;
}
diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
index 17bd020..d5f5ded 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -32,11 +32,6 @@
# define LINUX_SYSFS_SCSI_HOST_POSTFIX device
# define LINUX_SYSFS_FC_HOST_PREFIX /sys/class/fc_host/
-# define VPORT_CREATE 0
-# define VPORT_DELETE 1
-# define LINUX_SYSFS_VPORT_CREATE_POSTFIX /vport_create
-# define
[libvirt] [PATCH 03/12] nodedev: Implement virNodeDeviceFindByWWN
This just simply changes nodeDeviceFindByWWN to be not static,
and use that for udev and HAL backends.
---
src/node_device/node_device_driver.c |9 ++---
src/node_device/node_device_driver.h |4
src/node_device/node_device_hal.c|1 +
src/node_device/node_device_udev.c |1 +
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 522af99..6134507 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -224,10 +224,11 @@ cleanup:
}
-static virNodeDevicePtr
+virNodeDevicePtr
nodeDeviceLookupByWWN(virConnectPtr conn,
const char *wwnn,
- const char *wwpn)
+ const char *wwpn,
+ unsigned int flags)
{
unsigned int i;
virDeviceMonitorStatePtr driver = conn-devMonPrivateData;
@@ -236,6 +237,8 @@ nodeDeviceLookupByWWN(virConnectPtr conn,
virNodeDeviceObjPtr obj = NULL;
virNodeDevicePtr dev = NULL;
+virCheckFlags(0, NULL);
+
nodeDeviceLock(driver);
for (i = 0; i devs-count; i++) {
@@ -546,7 +549,7 @@ find_new_device(virConnectPtr conn, const char *wwnn, const
char *wwpn)
virFileWaitForDevices();
-dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn);
+dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn, 0);
if (dev != NULL) {
break;
diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
index 4cec07c..26f0550 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -77,6 +77,10 @@ int nodeListAllNodeDevices(virConnectPtr conn,
virNodeDevicePtr **devices,
unsigned int flags);
virNodeDevicePtr nodeDeviceLookupByName(virConnectPtr conn, const char *name);
+virNodeDevicePtr nodeDeviceLookupByWWN(virConnectPtr conn,
+ const char *wwnn,
+ const char *wwpn,
+ unsigned int flags);
char *nodeDeviceGetXMLDesc(virNodeDevicePtr dev, unsigned int flags);
char *nodeDeviceGetParent(virNodeDevicePtr dev);
int nodeDeviceNumOfCaps(virNodeDevicePtr dev);
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 610df8d..33e0170 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -765,6 +765,7 @@ static virDeviceMonitor halDeviceMonitor = {
.listDevices = nodeListDevices, /* 0.5.0 */
.listAllNodeDevices = nodeListAllNodeDevices, /* 0.10.2 */
.deviceLookupByName = nodeDeviceLookupByName, /* 0.5.0 */
+.deviceLookupByWWN = nodeDeviceLookupByWWN, /* 1.0.2 */
.deviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.5.0 */
.deviceGetParent = nodeDeviceGetParent, /* 0.5.0 */
.deviceNumOfCaps = nodeDeviceNumOfCaps, /* 0.5.0 */
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index a9b30b2..4137488 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1754,6 +1754,7 @@ static virDeviceMonitor udevDeviceMonitor = {
.listDevices = nodeListDevices, /* 0.7.3 */
.listAllNodeDevices = nodeListAllNodeDevices, /* 0.10.2 */
.deviceLookupByName = nodeDeviceLookupByName, /* 0.7.3 */
+.deviceLookupByWWN = nodeDeviceLookupByWWN, /* 1.0.2 */
.deviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.7.3 */
.deviceGetParent = nodeDeviceGetParent, /* 0.7.3 */
.deviceNumOfCaps = nodeDeviceNumOfCaps, /* 0.7.3 */
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/12] util: Add one helper virReadFCHost to read the value of fc_host entry
open_wwn_file in node_device_linux_sysfs.c is redundant, on one
hand it duplicates work of virFileReadAll, on the other hand, it's
waste to use a function for it, as there is no other users of it.
So I don't see why the file opening work cannot be done in
read_wwn_linux.
read_wwn_linux can be abstracted as an util function. As what all
it does is to read the sysfs entry.
So this patch removes open_wwn_file, and abstract read_wwn_linux
as an util function virReadFCHost (a more general name, because
after changes, it can read each of the fc_host entry now).
* src/util/virutil.h: (Declare virReadFCHost)
* src/util/virutil.c: (Implement virReadFCHost)
* src/node_device/node_device_linux_sysfs.c: (Remove open_wwn_file,
and read_wwn_linux)
src/node_device/node_device_driver.h: (Remove the declaration of
read_wwn_linux, and the related macros)
src/libvirt_private.syms: (Export virReadFCHost)
---
src/libvirt_private.syms |1 +
src/node_device/node_device_driver.h |4 -
src/node_device/node_device_linux_sysfs.c | 92 -
src/util/virutil.c| 67 +
src/util/virutil.h|5 ++
5 files changed, 85 insertions(+), 84 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9b7c1d..2e83747 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1297,6 +1297,7 @@ virIsDevMapperDevice;
virParseNumber;
virParseVersionString;
virPipeReadUntilEOF;
+virReadFCHost;
virScaleInteger;
virSetBlocking;
virSetCloseExec;
diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
index 26f0550..edd2915 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -59,14 +59,10 @@ int check_fc_host_linux(union _virNodeDevCapData *d);
# define check_vport_capable(d) check_vport_capable_linux(d)
int check_vport_capable_linux(union _virNodeDevCapData *d);
-# define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn)
-int read_wwn_linux(int host, const char *file, char **wwn);
-
# else /* __linux__ */
# define check_fc_host(d) (-1)
# define check_vport_capable(d)(-1)
-# define read_wwn(host, file, wwn)
# endif /* __linux__ */
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index 9c305d3..742a0bc 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -37,77 +37,6 @@
#ifdef __linux__
-static int open_wwn_file(const char *prefix,
- int host,
- const char *file,
- int *fd)
-{
-int retval = 0;
-char *wwn_path = NULL;
-
-if (virAsprintf(wwn_path, %s/host%d/%s, prefix, host, file) 0) {
-virReportOOMError();
-retval = -1;
-goto out;
-}
-
-/* fd will be closed by caller */
-if ((*fd = open(wwn_path, O_RDONLY)) != -1) {
-VIR_DEBUG(Opened WWN path '%s' for reading,
- wwn_path);
-} else {
-VIR_ERROR(_(Failed to open WWN path '%s' for reading),
- wwn_path);
-}
-
-out:
-VIR_FREE(wwn_path);
-return retval;
-}
-
-
-int read_wwn_linux(int host, const char *file, char **wwn)
-{
-char *p = NULL;
-int fd = -1, retval = 0;
-char buf[65] = ;
-
-if (open_wwn_file(LINUX_SYSFS_FC_HOST_PREFIX, host, file, fd) 0) {
-goto out;
-}
-
-if (saferead(fd, buf, sizeof(buf) - 1) 0) {
-retval = -1;
-VIR_DEBUG(Failed to read WWN for host%d '%s',
- host, file);
-goto out;
-}
-
-p = strstr(buf, 0x);
-if (p != NULL) {
-p += strlen(0x);
-} else {
-p = buf;
-}
-
-*wwn = strndup(p, sizeof(buf));
-if (*wwn == NULL) {
-virReportOOMError();
-retval = -1;
-goto out;
-}
-
-p = strchr(*wwn, '\n');
-if (p != NULL) {
-*p = '\0';
-}
-
-out:
-VIR_FORCE_CLOSE(fd);
-return retval;
-}
-
-
int check_fc_host_linux(union _virNodeDevCapData *d)
{
char *sysfs_path = NULL;
@@ -131,26 +60,29 @@ int check_fc_host_linux(union _virNodeDevCapData *d)
d-scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST;
-if (read_wwn(d-scsi_host.host,
- port_name,
- d-scsi_host.wwpn) == -1) {
+if (virReadFCHost(NULL,
+ d-scsi_host.host,
+ port_name,
+ d-scsi_host.wwpn) == -1) {
VIR_ERROR(_(Failed to read WWPN for host%d),
d-scsi_host.host);
retval = -1;
goto out;
}
-if (read_wwn(d-scsi_host.host,
- node_name,
- d-scsi_host.wwnn) == -1) {
+if (virReadFCHost(NULL,
+ d-scsi_host.host,
+ node_name,
+
[libvirt] [PATCH 01/12] Introduce new public API virNodeDeviceFindByWWN
Since the name (like scsi_host10) is not stable for vHBA, (it can
be changed either after recreating or system rebooting), current
API virNodeDeviceFindByName is not nice to use for management app
in this case. (E.g. one wants to destroy the vHBA whose name has
been changed after system rebooting, he has to find out current
name first).
Later patches will support the persistent vHBA via storage pool,
with which one can identify the vHBA stably by the wwnn wwpn
pair.
So this new API comes.
---
include/libvirt/libvirt.h.in |5
src/driver.h |6 +
src/libvirt.c| 45 ++
src/libvirt_public.syms |1 +
4 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 09c89c5..b4e1ead 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3156,6 +3156,11 @@ int virConnectListAllNodeDevices
(virConnectPtr conn,
virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn,
const char *name);
+virNodeDevicePtrvirNodeDeviceLookupByWWN (virConnectPtr conn,
+ const char *wwnn,
+ const char *wwpn,
+ unsigned int flags);
+
const char *virNodeDeviceGetName (virNodeDevicePtr dev);
const char *virNodeDeviceGetParent (virNodeDevicePtr dev);
diff --git a/src/driver.h b/src/driver.h
index 01c95cf..a4a6573 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1547,6 +1547,11 @@ typedef int (*virDevMonListAllNodeDevices)(virConnectPtr
conn,
typedef virNodeDevicePtr (*virDevMonDeviceLookupByName)(virConnectPtr conn,
const char *name);
+typedef virNodeDevicePtr (*virDevMonDeviceLookupByWWN)(virConnectPtr conn,
+ const char *wwnn,
+ const char *wwpn,
+ unsigned int flags);
+
typedef char * (*virDevMonDeviceGetXMLDesc)(virNodeDevicePtr dev,
unsigned int flags);
@@ -1578,6 +1583,7 @@ struct _virDeviceMonitor {
virDevMonListDeviceslistDevices;
virDevMonListAllNodeDevices listAllNodeDevices;
virDevMonDeviceLookupByName deviceLookupByName;
+virDevMonDeviceLookupByWWN deviceLookupByWWN;
virDevMonDeviceGetXMLDesc deviceGetXMLDesc;
virDevMonDeviceGetParentdeviceGetParent;
virDevMonDeviceNumOfCapsdeviceNumOfCaps;
diff --git a/src/libvirt.c b/src/libvirt.c
index 6d1da12..3ca7e9c 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -14303,6 +14303,51 @@ error:
return NULL;
}
+/**
+ * virNodeDeviceLookupByWWN:
+ * @conn: pointer to the hypervisor connection
+ * @wwnn: WWNN of the HBA.
+ * @wwpn: WWPN of the HBA.
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Lookup a node device (specially for HBA) by its WWNN and
+ * WWPN.
+ *
+ * Returns a virNodeDevicePtr if found, NULL otherwise.
+ */
+virNodeDevicePtr
+virNodeDeviceLookupByWWN(virConnectPtr conn,
+ const char *wwnn,
+ const char *wwpn,
+ unsigned int flags)
+{
+VIR_DEBUG(conn=%p, wwnn=%p, wwpn=%p, flags=%x, conn, wwnn, wwpn, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECT(conn)) {
+virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(NULL);
+return NULL;
+}
+
+virCheckNonNullArgGoto(wwnn, error);
+virCheckNonNullArgGoto(wwpn, error);
+
+if (conn-deviceMonitor conn-deviceMonitor-deviceLookupByWWN) {
+virNodeDevicePtr ret;
+ret = conn-deviceMonitor-deviceLookupByWWN(conn, wwnn, wwpn, flags);
+if (!ret)
+goto error;
+return ret;
+}
+
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(conn);
+return NULL;
+}
/**
* virNodeDeviceGetXMLDesc:
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 2107519..13f67ca 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -583,6 +583,7 @@ LIBVIRT_1.0.1 {
LIBVIRT_1.0.2 {
global:
virDomainOpenChannel;
+virNodeDeviceLookupByWWN;
} LIBVIRT_1.0.1;
# define new API here using predicted next version number
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/12] nodedev: Introduce two new flags for listAll API
VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST to filter the FC HBA,
and VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS to filter the FC HBA
which supports vport.
---
include/libvirt/libvirt.h.in | 20 +++-
src/conf/node_device_conf.c | 29 ++---
src/conf/node_device_conf.h |6 +-
src/libvirt.c|2 ++
tools/virsh-nodedev.c|6 ++
tools/virsh.pod |7 ---
6 files changed, 54 insertions(+), 16 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b4e1ead..019801a 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3138,15 +3138,17 @@ int virNodeListDevices
(virConnectPtr conn,
* type.
*/
typedef enum {
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM= 1 0, /* System
capability */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV = 1 1, /* PCI device */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV = 1 2, /* USB device */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE = 1 3, /* USB interface
*/
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET = 1 4, /* Network
device */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST = 1 5, /* SCSI Host Bus
Adapter */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET = 1 6, /* SCSI Target */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI = 1 7, /* SCSI device */
-VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE = 1 8, /* Storage
device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM= 1 0, /* System
capability */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV = 1 1, /* PCI device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV = 1 2, /* USB device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE = 1 3, /* USB
interface */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET = 1 4, /* Network
device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST = 1 5, /* SCSI Host
Bus Adapter */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET = 1 6, /* SCSI Target
*/
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI = 1 7, /* SCSI device
*/
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE = 1 8, /* Storage
device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST = 1 9, /* FC Host Bus
Adapter */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS= 1 10, /* Capable of
vport */
} virConnectListAllNodeDeviceFlags;
int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 48e8190..819e6af 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -50,7 +50,9 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST,
scsi_host,
scsi_target,
scsi,
- storage)
+ storage,
+ fc_host,
+ vports)
VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST,
80203,
@@ -467,8 +469,10 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
virBufferAddLit(buf,
capability type='hotpluggable' /\n);
break;
+case VIR_NODE_DEV_CAP_FC_HOST:
+case VIR_NODE_DEV_CAP_VPORTS:
case VIR_NODE_DEV_CAP_LAST:
-/* ignore special LAST value */
+default:
break;
}
@@ -1409,7 +1413,10 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
VIR_FREE(data-storage.serial);
VIR_FREE(data-storage.media_label);
break;
+case VIR_NODE_DEV_CAP_FC_HOST:
+case VIR_NODE_DEV_CAP_VPORTS:
case VIR_NODE_DEV_CAP_LAST:
+default:
/* This case is here to shutup the compiler */
break;
}
@@ -1437,6 +1444,18 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr devobj,
for (cap = devobj-def-caps; cap; cap = cap-next) {
if (type == cap-type)
return true;
+
+if (cap-type == VIR_NODE_DEV_CAP_SCSI_HOST) {
+if (type == VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST
+(cap-data.scsi_host.flags
+ VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST))
+return true;
+
+if (type == VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS
+(cap-data.scsi_host.flags
+ VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS))
+return true;
+}
}
return false;
@@ -1466,7 +1485,11 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
(MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI)
virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_SCSI))
||
(MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE)
- virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_STORAGE
+ virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_STORAGE))
||
+
[libvirt] [PATCH 00/12] Miscs of nodedev
Basicly, this set is created on the road to implement the supports for NPIV migration [1]. Some of them are preparations (such as the various new util functions), some of them are fixes, and others are improvements. Another set to support the persistent vHBA via scsi storage pool will come soon. Osier Yang (12): Introduce new public API virNodeDeviceFindByWWN remote: Wire up the remote protocol nodedev: Implement virNodeDeviceFindByWWN virsh: Use virNodeDeviceLookupByWWN nodedev: Remove the unused enum nodedev: Introduce two new flags for listAll API util: Add one helper virReadFCHost to read the value of fc_host entry nodedev: Use access instead of stat nodedev: Refactor the helpers nodedev: Dump max vports and vports in use for HBA's XML nodedev: Fix the improper logic when enumerating SRIOV VF nodedev: Abstract nodeDeviceVportCreateDelete as util function docs/formatnode.html.in | 10 +- docs/schemas/nodedev.rng |6 + include/libvirt/libvirt.h.in | 25 ++- src/conf/node_device_conf.c | 41 - src/conf/node_device_conf.h | 16 +- src/driver.h |6 + src/libvirt.c | 47 ++ src/libvirt_private.syms |4 + src/libvirt_public.syms |1 + src/node_device/node_device_driver.c | 119 ++ src/node_device/node_device_driver.h | 23 +-- src/node_device/node_device_hal.c | 16 ++- src/node_device/node_device_linux_sysfs.c | 245 +--- src/node_device/node_device_udev.c| 15 ++- src/remote/remote_driver.c|1 + src/remote/remote_protocol.x | 13 ++- src/remote_protocol-structs |9 + src/rpc/gendispatch.pl|4 +- src/util/virpci.c | 36 +++-- src/util/virutil.c| 224 ++ src/util/virutil.h| 19 +++ tools/virsh-nodedev.c | 97 +--- tools/virsh.pod | 22 ++- 23 files changed, 626 insertions(+), 373 deletions(-) https://www.redhat.com/archives/libvir-list/2012-November/msg00826.html Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] nodedev: Fix the improper logic when enumerating SRIOV VF
pciGetVirtualFunctions returns 0 even if there is no virtfn
entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions) ||
data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with virtual_function cap.
However, this results in a VF will aslo get virtual_function cap.
This patch fixes it by:
* Ignoring the VF which has failure of getting PCI config space
(given that the successfully detected VFs are kept , it makes
sense to not give up on the failure of one VF too) with a warning,
so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */
int rc = pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions);
if (ret 0 )
goto out;
else if (!ret (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
---
src/node_device/node_device_hal.c | 11 ---
src/node_device/node_device_udev.c | 11 ---
src/util/virpci.c | 36 +++-
3 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 18cdab8..b7f2d02 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -151,10 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char
*udi,
if (!pciGetPhysicalFunction(sysfs_path, d-pci_dev.physical_function))
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(sysfs_path, d-pci_dev.virtual_functions,
-d-pci_dev.num_virtual_functions) ||
-d-pci_dev.num_virtual_functions 0)
+int ret = pciGetVirtualFunctions(sysfs_path,
+ d-pci_dev.virtual_functions,
+ d-pci_dev.num_virtual_functions);
+if (ret 0) {
+VIR_FREE(sysfs_path);
+return -1;
+} else if (!ret (d-pci_dev.num_virtual_functions 0)) {
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
+}
VIR_FREE(sysfs_path);
}
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 87a2f61..73d0cf5 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device,
union _virNodeDevCapData *data = def-caps-data;
int ret = -1;
char *p;
+int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device,
if (!pciGetPhysicalFunction(syspath, data-pci_dev.physical_function))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(syspath, data-pci_dev.virtual_functions,
-data-pci_dev.num_virtual_functions) ||
-data-pci_dev.num_virtual_functions 0)
+rc = pciGetVirtualFunctions(syspath,
+data-pci_dev.virtual_functions,
+data-pci_dev.num_virtual_functions);
+/* Out of memory */
+if (rc 0)
+goto out;
+else if (!rc (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 4b26452..1e087e7 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1990,6 +1990,7 @@ pciGetVirtualFunctions(const char *sysfs_path,
unsigned int *num_virtual_functions)
{
int ret = -1;
+int i;
DIR *dir = NULL;
struct dirent *entry = NULL;
char *device_link = NULL;
@@ -2011,6 +2012,7 @@ pciGetVirtualFunctions(const char *sysfs_path,
*num_virtual_functions = 0;
while ((entry = readdir(dir))) {
if (STRPREFIX(entry-d_name, virtfn)) {
+struct pci_config_address *config_addr = NULL;
if (virBuildPath(device_link, sysfs_path, entry-d_name) == -1) {
virReportOOMError();
@@ -2019,24 +2021,23 @@ pciGetVirtualFunctions(const char *sysfs_path,
VIR_DEBUG(Number of virtual functions: %d,
*num_virtual_functions);
-if (VIR_REALLOC_N(*virtual_functions,
-
[libvirt] [PATCH 08/12] nodedev: Use access instead of stat
The use of 'stat' in nodeDeviceVportCreateDelete is only to check
if the file exists or not, it's a bit overkill, and safe to replace
with the wrapper of access(2) (virFileExists).
---
src/node_device/node_device_driver.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 6134507..ef85af5 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -28,7 +28,6 @@
#include errno.h
#include fcntl.h
#include time.h
-#include sys/stat.h
#include virerror.h
#include datatypes.h
@@ -422,7 +421,6 @@ nodeDeviceVportCreateDelete(const int parent_host,
int retval = 0;
char *operation_path = NULL, *vport_name = NULL;
const char *operation_file = NULL;
-struct stat st;
switch (operation) {
case VPORT_CREATE:
@@ -450,7 +448,7 @@ nodeDeviceVportCreateDelete(const int parent_host,
goto cleanup;
}
-if (stat(operation_path, st) != 0) {
+if (!virFileExists(operation_path)) {
VIR_FREE(operation_path);
if (virAsprintf(operation_path,
%shost%d%s,
@@ -462,7 +460,7 @@ nodeDeviceVportCreateDelete(const int parent_host,
goto cleanup;
}
-if (stat(operation_path, st) != 0) {
+if (!virFileExists(operation_path)) {
VIR_ERROR(_(No vport operation path found for host%d),
parent_host);
retval = -1;
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] remote: Wire up the remote protocol
Like virNodeDeviceFindByName, virNodeDeviceFindByWWN has to be
treated specially when generating the RPC codes. Also new rule
is added in fixup_name to keep the name FindByWWN.
---
src/remote/remote_driver.c |1 +
src/remote/remote_protocol.x | 13 -
src/remote_protocol-structs |9 +
src/rpc/gendispatch.pl |4 +++-
4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index c078cb5..3c0aeaf 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6274,6 +6274,7 @@ static virDeviceMonitor dev_monitor = {
.listDevices = remoteNodeListDevices, /* 0.5.0 */
.listAllNodeDevices = remoteConnectListAllNodeDevices, /* 0.10.2 */
.deviceLookupByName = remoteNodeDeviceLookupByName, /* 0.5.0 */
+.deviceLookupByWWN = remoteNodeDeviceLookupByWWN, /* 1.0.2 */
.deviceGetXMLDesc = remoteNodeDeviceGetXMLDesc, /* 0.5.0 */
.deviceGetParent = remoteNodeDeviceGetParent, /* 0.5.0 */
.deviceNumOfCaps = remoteNodeDeviceNumOfCaps, /* 0.5.0 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9035776..ccdaea2 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1852,6 +1852,16 @@ struct remote_node_device_lookup_by_name_ret {
remote_nonnull_node_device dev;
};
+struct remote_node_device_lookup_by_wwn_args {
+remote_nonnull_string wwnn;
+remote_nonnull_string wwpn;
+unsigned int flags;
+};
+
+struct remote_node_device_lookup_by_wwn_ret {
+remote_nonnull_node_device dev;
+};
+
struct remote_node_device_get_xml_desc_args {
remote_nonnull_string name;
unsigned int flags;
@@ -3049,7 +3059,8 @@ enum remote_procedure {
REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_FSTRIM = 294, /* autogen autogen */
REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, /* autogen autogen */
-REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296 /* autogen autogen | readstream@2 */
+REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296, /* autogen autogen | readstream@2 */
+REMOTE_PROC_NODE_DEVICE_LOOKUP_BY_WWN = 297 /* autogen autogen
priority:high */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 91414d4..368833a 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1400,6 +1400,14 @@ struct remote_node_device_lookup_by_name_args {
struct remote_node_device_lookup_by_name_ret {
remote_nonnull_node_device dev;
};
+struct remote_node_device_lookup_by_wwn_args {
+remote_nonull_string wwnn;
+remote_nonull_string wwpn;
+unsigned int flags;
+};
+struct remote_node_device_lookup_by_wwn_ret {
+remote_nonnull_node_device dev;
+};
struct remote_node_device_get_xml_desc_args {
remote_nonnull_string name;
u_int flags;
@@ -2453,4 +2461,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_FSTRIM = 294,
REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295,
REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296,
+REMOTE_PROC_NODE_DEVICE_LOOKUP_BY_WWN = 297,
};
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 899f4bc..1f895ba 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -45,6 +45,7 @@ sub fixup_name {
$name =~ s/Nmi$/NMI/;
$name =~ s/Pm/PM/;
$name =~ s/Fstrim$/FSTrim/;
+$name =~ s/Wwn$/WWN/;
return $name;
}
@@ -402,7 +403,8 @@ elsif ($opt_b) {
# node device is special, as it's identified by name
if ($argtype =~ m/^remote_node_device_/ and
!($argtype =~ m/^remote_node_device_lookup_by_name_/) and
-!($argtype =~ m/^remote_node_device_create_xml_/)) {
+!($argtype =~ m/^remote_node_device_create_xml_/) and
+!($argtype =~ m/^remote_node_device_lookup_by_wwn_/)) {
$has_node_device = 1;
push(@vars_list, virNodeDevicePtr dev = NULL);
push(@getters_list,
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Resolve FORWARD_NULL errors found by Coverity
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=880388 This set of patches resolves Error: FORWARD_NULL (CWE-476) errors found by Coverity. John Ferlan (6): xen: Avoid possible NULL dereference vmware: Avoid NULL dereference for 'caps' remote: Avoid calling virAuthConfigLookup() if 'credname' is NULL. lxc: Check for non NULL usb prior to calling usbFreeDevice() lxc: Avoid possible NULL dereference on *root prior to opendir(). cpu: Avoid NULL dereference src/cpu/cpu_powerpc.c | 3 ++- src/lxc/lxc_container.c| 6 ++ src/lxc/lxc_driver.c | 6 -- src/remote/remote_driver.c | 3 ++- src/vmware/vmware_conf.c | 3 ++- src/xen/xen_driver.c | 6 +++--- 6 files changed, 19 insertions(+), 8 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] lxc: Check for non NULL usb prior to calling usbFreeDevice()
--- src/lxc/lxc_driver.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8050ce6..d4b4989 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3468,7 +3468,8 @@ cleanup: if (ret 0 created) unlink(dstfile); -usbFreeDevice(usb); +if (usb) +usbFreeDevice(usb); virCgroupFree(group); VIR_FREE(src); VIR_FREE(dstfile); @@ -4022,7 +4023,8 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, ret = 0; cleanup: -usbFreeDevice(usb); +if (usb) +usbFreeDevice(usb); VIR_FREE(dst); virCgroupFree(group); return ret; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] vmware: Avoid NULL dereference for 'caps'
When virCapabilitiesNew() fails, caps will be NULL resulting in possible core when deref'd in cpuDataFree() call. --- src/vmware/vmware_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index b514636..fd9c473 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -127,7 +127,8 @@ vmwareCapsInit(void) cleanup: virCPUDefFree(cpu); -cpuDataFree(caps-host.arch, data); +if (caps) +cpuDataFree(caps-host.arch, data); return caps; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] remote: Avoid calling virAuthConfigLookup() if 'credname' is NULL.
--- src/remote/remote_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c078cb5..65fcc5b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3738,7 +3738,8 @@ static int remoteAuthFillFromConfig(virConnectPtr conn, break; } -if (virAuthConfigLookup(state-config, +if (credname +virAuthConfigLookup(state-config, libvirt, VIR_URI_SERVER(conn-uri), credname, -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] xen: Avoid possible NULL dereference
Change calling sequence to only call xenUnifiedDomainSetVcpusFlags() when
'dom' is not NULL. Use the GET_PRIVATE() macro to reference privateData.
Just return -1 if dom is NULL.
---
src/xen/xen_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 2ef2c57..559025e 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1253,17 +1253,17 @@ static int
xenUnifiedDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
{
unsigned int flags = VIR_DOMAIN_VCPU_LIVE;
-xenUnifiedPrivatePtr priv;
/* Per the documented API, it is hypervisor-dependent whether this
* affects just _LIVE or _LIVE|_CONFIG; in xen's case, that
* depends on xendConfigVersion. */
if (dom) {
-priv = dom-conn-privateData;
+GET_PRIVATE(dom-conn);
if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
flags |= VIR_DOMAIN_VCPU_CONFIG;
+return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
}
-return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
+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 6/6] cpu: Avoid NULL dereference
Don't dereference 'model' in PowerPCBaseline when there's no outputModel --- src/cpu/cpu_powerpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 8bef627..5e1a7b9 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -605,7 +605,8 @@ PowerPCBaseline(virCPUDefPtr *cpus, goto error; } -base_model-data-ppc.pvr = model-data-ppc.pvr; +if (outputModel) +base_model-data-ppc.pvr = model-data-ppc.pvr; if (PowerPCDecode(cpu, base_model-data, models, nmodels, NULL) 0) goto error; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] lxc: Avoid possible NULL dereference on *root prior to opendir().
If running on older Linux without mounted cgroups then its possible that
*root would be NULL.
---
src/lxc/lxc_container.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index d3a2968..d234426 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1762,6 +1762,12 @@ static int lxcContainerIdentifyCGroups(struct
lxcContainerCGroup **mountsret,
VIR_DEBUG(Grabbed '%s', mntent.mnt_dir);
}
+if (!*root) {
+VIR_DEBUG(No mounted cgroups found);
+ret = 0;
+goto cleanup;
+}
+
VIR_DEBUG(Checking for symlinks in %s, *root);
if (!(dh = opendir(*root))) {
virReportSystemError(errno,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] nodedev: Remove the unused enum
Guess it was created for the fc_host and vports_ops capabilities
purpose, but there is enum virNodeDevScsiHostCapFlags for them,
and enum virNodeDevHBACapType is unused, and actually both
VIR_ENUM_DECL and VIR_ENUM_IMPL use the wrong enum name
virNodeDevHBACap.
---
src/conf/node_device_conf.c |5 -
src/conf/node_device_conf.h |8
2 files changed, 0 insertions(+), 13 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 53b6af2..48e8190 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -56,11 +56,6 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST,
80203,
80211)
-VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST,
- fc_host,
- vport_ops)
-
-
static int
virNodeDevCapsDefParseString(const char *xpath,
xmlXPathContextPtr ctxt,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 12c36d8..36bf5ac 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -55,16 +55,8 @@ enum virNodeDevNetCapType {
VIR_NODE_DEV_CAP_NET_LAST
};
-enum virNodeDevHBACapType {
-/* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */
-VIR_NODE_DEV_CAP_HBA_FC_HOST, /* fibre channel HBA */
-VIR_NODE_DEV_CAP_HBA_VPORT_OPS,/* capable of vport operations */
-VIR_NODE_DEV_CAP_HBA_LAST
-};
-
VIR_ENUM_DECL(virNodeDevCap)
VIR_ENUM_DECL(virNodeDevNetCap)
-VIR_ENUM_DECL(virNodeDevHBACap)
enum virNodeDevStorageCapFlags {
VIR_NODE_DEV_CAP_STORAGE_REMOVABLE = (1 0),
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 2/2] S390: Enable SCLP Console in QEMU driver
From: J.B. Joret j...@linux.vnet.ibm.com
This is the QEMU backend code for the SCLP console support.
It includes SCLP capability detection, QEMU command line generation
and a test case.
Signed-off-by: J.B. Joret j...@linux.vnet.ibm.com
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
src/qemu/qemu_capabilities.c |3 +
src/qemu/qemu_capabilities.h |1 +
src/qemu/qemu_command.c| 59
.../qemuxml2argv-console-sclp.args |8 +++
.../qemuxml2argvdata/qemuxml2argv-console-sclp.xml | 24
tests/qemuxml2argvtest.c |3 +
6 files changed, 98 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.xml
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e16bc70..be78461 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -199,6 +199,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
cirrus-vga,
vmware-svga,
device-video-primary,
+ s390-sclp,
+
);
struct _qemuCaps {
@@ -1331,6 +1333,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = {
{ usb-hub, QEMU_CAPS_USB_HUB },
{ ich9-ahci, QEMU_CAPS_ICH9_AHCI },
{ virtio-blk-s390, QEMU_CAPS_VIRTIO_S390 },
+{ sclpconsole, QEMU_CAPS_SCLP_S390 },
{ lsi53c895a, QEMU_CAPS_SCSI_LSI },
{ virtio-scsi-pci, QEMU_CAPS_VIRTIO_SCSI_PCI },
{ spicevmc, QEMU_CAPS_DEVICE_SPICEVMC },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 1417d7f..399212b 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_SCLP_S390 = 124, /* -device sclp* */
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 8a3de09..92fc6b4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4132,6 +4132,37 @@ error:
return NULL;
}
+static char *qemuBuildSclpDevStr(virDomainChrDefPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE) {
+switch (dev-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+virBufferAddLit(buf, sclpconsole);
+break;
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+virBufferAddLit(buf, sclplmconsole);
+break;
+}
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Cannot use slcp with devices other than console));
+goto error;
+}
+virBufferAsprintf(buf, ,chardev=char%s,id=%s,
+ dev-info.alias, dev-info.alias);
+if (virBufferError(buf)) {
+virReportOOMError();
+goto error;
+}
+
+return virBufferContentAndReset(buf);
+
+error:
+virBufferFreeAndReset(buf);
+return NULL;
+}
+
static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -6383,6 +6414,34 @@ qemuBuildCommandLine(virConnectPtr conn,
char *devstr;
switch (console-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(sclp console requires QEMU to support
-device));
+goto error;
+}
+if (!qemuCapsGet(caps, QEMU_CAPS_SCLP_S390)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(sclp console requires QEMU to support
s390-sclp));
+goto error;
+}
+
+virCommandAddArg(cmd, -chardev);
+if (!(devstr = qemuBuildChrChardevStr(console-source,
+ console-info.alias,
+ caps)))
+goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+
+virCommandAddArg(cmd, -device);
+if (!(devstr = qemuBuildSclpDevStr(console)))
+goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+break;
+
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
diff --git
[libvirt] [PATCHv4 1/2] S390: Add SCLP console front end support
From: J.B. Joret j...@linux.vnet.ibm.com
The SCLP console is the native console type for s390 and is preferred
over the virtio console as it doesn't require special drivers and
is more efficient. Recent versions of QEMU come with SCLP support
which is hereby enabled.
The new target types 'sclp' and 'sclplm' can be used to specify a
SCLP console. Adding documentation, domain schema and XML processing
support.
Signed-off-by: J.B. Joret j...@linux.vnet.ibm.com
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
docs/formatdomain.html.in | 19 ++-
docs/schemas/domaincommon.rng |2 ++
src/conf/domain_conf.c|4 +++-
src/conf/domain_conf.h|2 ++
4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 977497a..c5725e5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3701,7 +3701,7 @@ qemu-kvm -net nic,model=? /dev/null
/p
ul
- liIf no codetargetType/code attribue is set, then the default
+ liIf no codetargetType/code attribute is set, then the default
device type is according to the hypervisor's rules. The default
type will be added when re-querying the XML fed into libvirt.
For fully virtualized guests, the default device type will usually
@@ -3715,6 +3715,12 @@ qemu-kvm -net nic,model=? /dev/null
liOnly the first codeconsole/code element may use a
codetargetType/code
of codeserial/code. Secondary consoles must all be paravirtualized.
/li
+ liOn s390, the codeconsole/code element may use a
+codetargetType/code of codesclp/code or codesclplm/code
+(line mode). SCLP is the native console type for s390. There's no
+controller associated to SCLP consoles.
+span class=sinceSince 1.0.2/span
+ /li
/ul
p
@@ -3740,6 +3746,17 @@ qemu-kvm -net nic,model=? /dev/null
lt;/devicesgt;
.../pre
+pre
+ ...
+ lt;devicesgt;
+lt;!-- KVM s390 sclp console --gt;
+lt;console type='pty'gt;
+ lt;source path='/dev/pts/1'/gt;
+ lt;target type='sclp' port='0'/gt;
+lt;/consolegt;
+ lt;/devicesgt;
+ .../pre
+
p
If the console is presented as a serial port, the codetarget/code
element has the same attributes as for a serial port. There is usually
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7a97781..be12102 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2467,6 +2467,8 @@
valuevirtio/value
valuelxc/value
valueopenvz/value
+valuesclp/value
+valuesclplm/value
/choice
/attribute
/define
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b2bc57a..b3a35b7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -355,7 +355,9 @@ VIR_ENUM_IMPL(virDomainChrConsoleTarget,
uml,
virtio,
lxc,
- openvz)
+ openvz,
+ sclp,
+ sclplm)
VIR_ENUM_IMPL(virDomainChrDevice, VIR_DOMAIN_CHR_DEVICE_TYPE_LAST,
parallel,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d825a24..4284caf 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -933,6 +933,8 @@ enum virDomainChrConsoleTargetType {
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO,
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC,
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ,
+VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP,
+VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM,
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST
};
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Speed up fallback to legacy non-QMP probing
On 01/07/2013 09:20 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Since we daemonized QEMU for capabilities probing there is a long time if QEMU fals to launch. This is because we're not passing in s/fals/fails/ any virDomainObjPtr instance and thus the monitor code can not check to see if the PID is still alive. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/12] Introduce new public API virNodeDeviceFindByWWN
On Tue, Jan 08, 2013 at 01:05:23AM +0800, Osier Yang wrote: Since the name (like scsi_host10) is not stable for vHBA, (it can be changed either after recreating or system rebooting), current API virNodeDeviceFindByName is not nice to use for management app in this case. (E.g. one wants to destroy the vHBA whose name has been changed after system rebooting, he has to find out current name first). Later patches will support the persistent vHBA via storage pool, with which one can identify the vHBA stably by the wwnn wwpn pair. So this new API comes. --- include/libvirt/libvirt.h.in |5 src/driver.h |6 + src/libvirt.c| 45 ++ src/libvirt_public.syms |1 + 4 files changed, 57 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 09c89c5..b4e1ead 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3156,6 +3156,11 @@ int virConnectListAllNodeDevices (virConnectPtr conn, virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn, const char *name); +virNodeDevicePtrvirNodeDeviceLookupByWWN (virConnectPtr conn, + const char *wwnn, + const char *wwpn, + unsigned int flags); Since this API doesn't work for all types of node dev, its name should really reflect that. eg virNodeDeviceLookupSCSIHostByWWN() 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] [PATCHv4 0/2] S390: Adding support for SCLP Console
The S390 architecture comes with a native console type (SCLP console) which is now also supported by current QEMU. This series is enabling libvirt to configure S390 domains with SCLP consoles. The domain XML has to be extended for the new console target types 'sclp' and 'sclplm' (line mode = dumb). As usual the QEMU driver must do capability probing in order to find out whether SCLP is supported and format the QEMU command line for the new console type. V2/V3 Changes: Rebased to current master, resolving conflicts. V4 Changes: Rebased, changed 'since' to 1.0.2 in formatdomain.html.in J.B. Joret (2): S390: Add SCLP console front end support S390: Enable SCLP Console in QEMU driver docs/formatdomain.html.in | 19 ++- docs/schemas/domaincommon.rng |2 + src/conf/domain_conf.c |4 +- src/conf/domain_conf.h |2 + src/qemu/qemu_capabilities.c |3 + src/qemu/qemu_capabilities.h |1 + src/qemu/qemu_command.c| 59 .../qemuxml2argv-console-sclp.args |8 +++ .../qemuxml2argvdata/qemuxml2argv-console-sclp.xml | 24 tests/qemuxml2argvtest.c |3 + 10 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.xml -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Only initialize capabilities after setting dir permissions
On 01/07/2013 09:21 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The current code is initializing capabilities before setting directory permissions. Thus the QEMU binaries being run may not have the ability to create the UNIX monitor socket on the first run of libvirtd. --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Convert libxl driver to Xen 4.2
On 12/06/2012 03:18 PM, Eric Blake wrote: On 11/30/2012 03:13 PM, Jim Fehlig wrote: Based on a patch originally authored by Daniel De Graaf http://lists.xen.org/archives/html/xen-devel/2012-05/msg00565.html This patch converts the Xen libxl driver to support only Xen = 4.2. Support for Xen 4.1 libxl is dropped since that version of libxl is designated 'technology preview' only and is incompatible with Xen 4.2 libxl. Additionally, the default toolstack in Xen 4.1 is still xend, for which libvirt has a stable, functional driver. --- V2: Remove 128 vcpu limit. Remove split_string_into_string_list() function copied from xen sources since libvirt now has virStringSplit(). Tested on Fedora 18, with its use of xen 4.2. ACK; let's get this pushed. I've also backported this to v0.10.2-maint, since that is the branch used for F18 libvirt. I had some merge conflicts to resolve, so I'll reply to this message with the actual patches that I pushed, if anyone is interested in double-checking them. I went ahead and pushed the backports under the build-breaker rule (as it was interfering with me backporting some other patches to v0.10.2-maint on my F18 box). -- 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 0.10.2-maint 1/2] Introduce APIs for splitting/joining strings
From: Daniel P. Berrange berra...@redhat.com
This introduces a few new APIs for dealing with strings.
One to split a char * into a char **, another to join a
char ** into a char *, and finally one to free a char **
There is a simple test suite to validate the edge cases
too. No more need to use the horrible strtok_r() API,
or hand-written code for splitting strings.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
(cherry picked from commit 76c1fd33c8093d6a7173a85486e1e6f51a832135)
Conflicts:
tests/Makefile.am - commit eca72d4 not backported
---
.gitignore | 1 +
src/Makefile.am | 1 +
src/libvirt_private.syms | 6 ++
src/util/virstring.c | 168 +++
src/util/virstring.h | 38 +++
tests/Makefile.am| 9 ++-
tests/virstringtest.c| 161 +
7 files changed, 383 insertions(+), 1 deletion(-)
create mode 100644 src/util/virstring.c
create mode 100644 src/util/virstring.h
create mode 100644 tests/virstringtest.c
diff --git a/.gitignore b/.gitignore
index 004bc76..6db09ef 100644
--- a/.gitignore
+++ b/.gitignore
@@ -167,6 +167,7 @@
/tests/virkeyfiletest
/tests/virnet*test
/tests/virshtest
+/tests/virstringtest
/tests/virtimetest
/tests/viruritest
/tests/vmx2xmltest
diff --git a/src/Makefile.am b/src/Makefile.am
index ae3d491..f82c710 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -110,6 +110,7 @@ UTIL_SOURCES =
\
util/virnetlink.c util/virnetlink.h \
util/virrandom.h util/virrandom.c \
util/virsocketaddr.h util/virsocketaddr.c \
+ util/virstring.h util/virstring.c \
util/virtime.h util/virtime.c \
util/viruri.h util/viruri.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 945894a..45cdace 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1761,6 +1761,12 @@ virSetErrorLogPriorityFunc;
virStrerror;
+# virstring.h
+virStringSplit;
+virStringJoin;
+virStringFreeList;
+
+
# virtime.h
virTimeFieldsNow;
virTimeFieldsNowRaw;
diff --git a/src/util/virstring.c b/src/util/virstring.c
new file mode 100644
index 000..1917e9a
--- /dev/null
+++ b/src/util/virstring.c
@@ -0,0 +1,168 @@
+/*
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Authors:
+ * Daniel P. Berrange berra...@redhat.com
+ */
+
+#include config.h
+
+#include virstring.h
+#include memory.h
+#include buf.h
+#include virterror_internal.h
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+/*
+ * The following virStringSplit virStringJoin methods
+ * are derived from g_strsplit / g_strjoin in glib2,
+ * also available under the LGPLv2+ license terms
+ */
+
+/**
+ * virStringSplit:
+ * @string: a string to split
+ * @delim: a string which specifies the places at which to split
+ * the string. The delimiter is not included in any of the resulting
+ * strings, unless @max_tokens is reached.
+ * @max_tokens: the maximum number of pieces to split @string into.
+ * If this is 0, the string is split completely.
+ *
+ * Splits a string into a maximum of @max_tokens pieces, using the given
+ * @delim. If @max_tokens is reached, the remainder of @string is
+ * appended to the last token.
+ *
+ * As a special case, the result of splitting the empty string is an empty
+ * vector, not a vector containing a single string. The reason for this
+ * special case is that being able to represent a empty vector is typically
+ * more useful than consistent handling of empty elements. If you do need
+ * to represent empty elements, you'll need to check for the empty string
+ * before calling virStringSplit().
+ *
+ * Return value: a newly-allocated NULL-terminated array of strings. Use
+ *virStringFreeList() to free it.
+ */
+char **virStringSplit(const char *string,
+ const char *delim,
+ size_t max_tokens)
+{
+char **tokens = NULL;
+size_t ntokens = 0;
+size_t maxtokens = 0;
+const char *remainder = string;
+char *tmp;
+size_t i;
+
+if (max_tokens == 0)
+max_tokens = INT_MAX;
+
+tmp = strstr(remainder, delim);
+if
[libvirt] [PATCH] build: Add libxenctrl to LIBXL_LIBS
Commit dfa1e1dd removed libxenctrl from LIBXL_LIBS, but the libxl driver uses a symbol from this library. Explicitly link with libxenctrl instead of relying on the build system to support implicit DSO linking. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ab08f17..20108c2 100644 --- a/configure.ac +++ b/configure.ac @@ -733,7 +733,7 @@ if test $with_libxl != no ; then LIBS=$LIBS $LIBXL_LIBS AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [ with_libxl=yes -LIBXL_LIBS=$LIBXL_LIBS -lxenlight +LIBXL_LIBS=$LIBXL_LIBS -lxenlight -lxenctrl ],[ if test $with_libxl = yes; then fail=1 -- 1.8.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Add libxenctrl to LIBXL_LIBS
On 01/07/2013 10:40 AM, Jim Fehlig wrote: Commit dfa1e1dd removed libxenctrl from LIBXL_LIBS, but the libxl driver uses a symbol from this library. Explicitly link with libxenctrl instead of relying on the build system to support implicit DSO linking. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ab08f17..20108c2 100644 --- a/configure.ac +++ b/configure.ac @@ -733,7 +733,7 @@ if test $with_libxl != no ; then LIBS=$LIBS $LIBXL_LIBS AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [ with_libxl=yes -LIBXL_LIBS=$LIBXL_LIBS -lxenlight +LIBXL_LIBS=$LIBXL_LIBS -lxenlight -lxenctrl ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Add libxenctrl to LIBXL_LIBS
Eric Blake wrote: On 01/07/2013 10:40 AM, Jim Fehlig wrote: Commit dfa1e1dd removed libxenctrl from LIBXL_LIBS, but the libxl driver uses a symbol from this library. Explicitly link with libxenctrl instead of relying on the build system to support implicit DSO linking. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ab08f17..20108c2 100644 --- a/configure.ac +++ b/configure.ac @@ -733,7 +733,7 @@ if test $with_libxl != no ; then LIBS=$LIBS $LIBXL_LIBS AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [ with_libxl=yes -LIBXL_LIBS=$LIBXL_LIBS -lxenlight +LIBXL_LIBS=$LIBXL_LIBS -lxenlight -lxenctrl ACK. Thanks, pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 00/11] disable-kvm_mmu + -cpu check/enforce fixes (v2)
Am 04.01.2013 23:01, schrieb Eduardo Habkost: Eduardo Habkost (11): [...] target-i386: kvm: -cpu host: Use GET_SUPPORTED_CPUID for SVM features target-i386: kvm: Enable all supported KVM features for -cpu host target-i386: check/enforce: Fix CPUID leaf numbers on error messages target-i386: check/enforce: Do not ignore hypervisor flag target-i386: check/enforce: Check all CPUID.8001H.EDX bits target-i386: check/enforce: Check SVM flag support as well target-i386: check/enforce: Eliminate check_feat field [snip] Thanks, applied patches 3-9 to qom-cpu queue (fixing some typos in commit messages): https://github.com/afaerber/qemu-cpu/commits/qom-cpu Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ensure we always setup a private mount namespace for LXC controller
From: Daniel P. Berrange berra...@redhat.com
The code for setting up a private /dev/pts for the containers
is also responsible for making the LXC controller have a
private mount namespace. Unfortunately the /dev/pts code is
not run if launching a container without a custom root. This
causes the LXC FUSE mount to leak into the host FS.
---
src/lxc/lxc_controller.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c9d96b3..a1c264c 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1143,6 +1143,29 @@ cleanup:
static int
+virLXCControllerSetupPrivateNS(void)
+{
+int ret = -1;
+
+if (unshare(CLONE_NEWNS) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot unshare mount namespace));
+goto cleanup;
+}
+
+if (mount(, /, NULL, MS_SLAVE|MS_REC, NULL) 0) {
+virReportSystemError(errno, %s,
+ _(Failed to switch root mount into slave mode));
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+return ret;
+}
+
+
+static int
virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl)
{
virDomainFSDefPtr root = virDomainGetRootFilesystem(ctrl-def);
@@ -1193,18 +1216,6 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl)
goto cleanup;
}
-if (unshare(CLONE_NEWNS) 0) {
-virReportSystemError(errno, %s,
- _(Cannot unshare mount namespace));
-goto cleanup;
-}
-
-if (mount(, /, NULL, MS_SLAVE|MS_REC, NULL) 0) {
-virReportSystemError(errno, %s,
- _(Failed to switch root mount into slave mode));
-goto cleanup;
-}
-
if (virAsprintf(devpts, %s/dev/pts, root-src) 0 ||
virAsprintf(ctrl-devptmx, %s/dev/pts/ptmx, root-src) 0) {
virReportOOMError();
@@ -1408,6 +1419,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
goto cleanup;
}
+if (virLXCControllerSetupPrivateNS() 0)
+goto cleanup;
+
if (virLXCControllerSetupLoopDevices(ctrl) 0)
goto cleanup;
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 4/7] target-i386/cpu: Introduce FeatureWord typedefs
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
+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,
-plus_ext_features, plus_ext2_features,
-plus_ext3_features, plus_kvm_features,
-plus_svm_features, plus_7_0_ebx_features);
+add_flagname_to_bitmaps(featurestr + 1, plus_features);
} else if
[libvirt] [PATCH] Fix virLXCPrepareHostDevices method
From: Daniel P. Berrange berra...@redhat.com
The virLXCPrepareHostDevices method was returning success even
when it reported an error, and failed to handle several host
device types
---
src/lxc/lxc_hostdev.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_hostdev.c b/src/lxc/lxc_hostdev.c
index 21f3096..4fa0508 100644
--- a/src/lxc/lxc_hostdev.c
+++ b/src/lxc/lxc_hostdev.c
@@ -299,15 +299,29 @@ int virLXCPrepareHostDevices(virLXCDriverPtr driver,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported hostdev type %s),
virDomainHostdevSubsysTypeToString(dev-source.subsys.type));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
+switch (dev-source.subsys.type) {
+case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE:
+case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC:
break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported hostdev type %s),
+
virDomainHostdevSubsysTypeToString(dev-source.subsys.type));
+return -1;
}
break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported hostdev mode %s),
virDomainHostdevModeTypeToString(dev-mode));
-break;
+return -1;
}
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 5/7] target-i386: kvm_check_features_against_host(): Use feature_word_info
Instead of carrying the CPUID leaf/register and feature name array on
the model_features_t struct, move that information into
feature_word_info so it can be reused by other functions.
The goal is to eventually kill model_features_t entirely, but to do that
we have to either convert x86_def_t.features to an array or use
offsetof() inside FeatureWordInfo (to replace the pointers inside
model_features_t). So by now just move most of the model_features_t
fields to FeatureWordInfo except for the two pointers to local
arguments.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 73 +--
1 file changed, 49 insertions(+), 24 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7d62d48..4b3ee63 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -126,16 +126,39 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
typedef struct FeatureWordInfo {
const char **feat_names;
+uint32_t cpuid_eax; /* Input EAX for CPUID */
+int cpuid_reg; /* R_* register constant */
} 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 },
+[FEAT_1_EDX] = {
+.feat_names = feature_name,
+.cpuid_eax = 1, .cpuid_reg = R_EDX,
+},
+[FEAT_1_ECX] = {
+.feat_names = ext_feature_name,
+.cpuid_eax = 1, .cpuid_reg = R_ECX,
+},
+[FEAT_8000_0001_EDX] = {
+.feat_names = ext2_feature_name,
+.cpuid_eax = 0x8001, .cpuid_reg = R_EDX,
+},
+[FEAT_8000_0001_ECX] = {
+.feat_names = ext3_feature_name,
+.cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
+},
+[FEAT_KVM] = {
+.feat_names = kvm_feature_name,
+.cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
+},
+[FEAT_SVM] = {
+.feat_names = svm_feature_name,
+.cpuid_eax = 0x800A, .cpuid_reg = R_EDX,
+},
+[FEAT_7_0_EBX] = {
+.feat_names = cpuid_7_0_ebx_feature_name,
+.cpuid_eax = 7, .cpuid_reg = R_EBX,
+},
};
const char *get_register_name_32(unsigned int reg)
@@ -162,9 +185,7 @@ const char *get_register_name_32(unsigned int reg)
typedef struct model_features_t {
uint32_t *guest_feat;
uint32_t *host_feat;
-const char **flag_names;
-uint32_t cpuid;
-int reg;
+FeatureWord feat_word;
} model_features_t;
int check_cpuid = 0;
@@ -935,19 +956,19 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
#endif /* CONFIG_KVM */
}
-static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
+static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
{
int i;
for (i = 0; i 32; ++i)
if (1 i mask) {
-const char *reg = get_register_name_32(f-reg);
+const char *reg = get_register_name_32(f-cpuid_reg);
assert(reg);
fprintf(stderr, warning: host doesn't support requested feature:
CPUID.%02XH:%s%s%s [bit %d]\n,
-f-cpuid, reg,
-f-flag_names[i] ? . : ,
-f-flag_names[i] ? f-flag_names[i] : , i);
+f-cpuid_eax, reg,
+f-feat_names[i] ? . : ,
+f-feat_names[i] ? f-feat_names[i] : , i);
break;
}
return 0;
@@ -965,25 +986,29 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
int rv, i;
struct model_features_t ft[] = {
{guest_def-features, host_def.features,
-feature_name, 0x0001, R_EDX},
+FEAT_1_EDX },
{guest_def-ext_features, host_def.ext_features,
-ext_feature_name, 0x0001, R_ECX},
+FEAT_1_ECX },
{guest_def-ext2_features, host_def.ext2_features,
-ext2_feature_name, 0x8001, R_EDX},
+FEAT_8000_0001_EDX },
{guest_def-ext3_features, host_def.ext3_features,
-ext3_feature_name, 0x8001, R_ECX}
+FEAT_8000_0001_ECX },
};
assert(kvm_enabled());
kvm_cpu_fill_host(host_def);
-for (rv = 0, i = 0; i ARRAY_SIZE(ft); ++i)
-for (mask = 1; mask; mask = 1)
+for (rv = 0, i = 0; i ARRAY_SIZE(ft); ++i) {
+FeatureWord w = ft[i].feat_word;
+FeatureWordInfo *wi = feature_word_info[w];
+for (mask = 1; mask; mask = 1) {
if (*ft[i].guest_feat mask
!(*ft[i].host_feat mask)) {
-unavailable_host_feature(ft[i], mask);
-rv = 1;
-}
+
[libvirt] [PATCH qom-cpu 0/7] disable kvm_mmu + -cpu enforce fixes (v3)
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. 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 7/7] target-i386: check/enforce: Check all feature words
This adds the following feature words to the list of flags to be checked
by kvm_check_features_against_host():
- cpuid_7_0_ebx_features
- ext4_features
- kvm_features
- svm_features
This will ensure the enforce flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.
This patch may cause existing configurations where enforce wasn't
preventing QEMU from being started to abort QEMU. But that's exactly the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the enforce code.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: Gleb Natapov g...@redhat.com
Cc: Marcelo Tosatti mtosa...@redhat.com
Cc: k...@vger.kernel.org
Cc: libvir-list@redhat.com
Cc: Jiri Denemark jdene...@redhat.com
CCing libvirt people, as this is directly related to the planned usage
of the enforce flag by libvirt.
The libvirt team probably has a problem in their hands: libvirt should
use enforce to make sure all requested flags are making their way into
the guest (so the resulting CPU is always the same, on any host), but
users may have existing working configurations where a flag is not
supported by the guest and the user really doesn't care about it. Those
configurations will necessarily break when libvirt starts using
enforce.
One example where it may cause trouble for common setups: pc-1.3 wants
the kvm_pv_eoi flag enabled by default (so enforce will make sure it
is enabled), but the user may have an existing VM running on a host
without pv_eoi support. That setup is unsafe today because
live-migration between different host kernel versions may enable/disable
pv_eoi silently (that's why we need the enforce flag to be used by
libvirt), but the user probably would like to be able to live-migrate
that VM anyway (and have libvirt to just do the right thing).
One possible solution to libvirt is to use enforce only on newer
machine-types, so existing machines with older machine-types will keep
the unsafe host-dependent-ABI behavior, but at least would keep
live-migration working in case the user is careful.
I really don't know what the libvirt team prefers, but that's the
situation today. 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.
Changes v2:
- Coding style fix
Changes v3:
- Added ext4_feature_name array
---
target-i386/cpu.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a54c464..68cabcf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -989,8 +989,9 @@ static int unavailable_host_feature(FeatureWordInfo *f,
uint32_t mask)
return 0;
}
-/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
*
* This function may be called only if KVM is enabled.
*/
@@ -1008,6 +1009,14 @@ static int kvm_check_features_against_host(x86_def_t
*guest_def)
FEAT_8000_0001_EDX },
{guest_def-ext3_features, host_def.ext3_features,
FEAT_8000_0001_ECX },
+{guest_def-ext4_features, host_def.ext4_features,
+FEAT_C000_0001_EDX },
+{guest_def-cpuid_7_0_ebx_features, host_def.cpuid_7_0_ebx_features,
+FEAT_7_0_EBX },
+{guest_def-svm_features, host_def.svm_features,
+FEAT_SVM },
+{guest_def-kvm_features, host_def.kvm_features,
+FEAT_KVM },
};
assert(kvm_enabled());
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 3/7] target-i386: Disable kvm_mmu by default
KVM_CAP_PV_MMU capability reporting was removed from the kernel since v2.6.33 (see commit a68a6a7282373), and was completely removed from the kernel since v3.3 (see commit fb92045843). It doesn't make sense to keep it enabled by default, as it would cause unnecessary hassle when using the enforce flag. This disables kvm_mmu on all machine-types. With this fix, the possible scenarios when migrating from QEMU = 1.3 to QEMU 1.4 are; ++ src kernel | dst kernel | Result ++ = 2.6.33 | any| kvm_mmu was already disabled and will stay disabled = 2.6.32 | = 3.3 | correct live migration is impossible = 2.6.32 | = 3.2 | kvm_mmu will be disabled on next guest reboot * ++ * If they are running kernel = 2.6.32 and want kvm_mmu to be kept enabled on guest reboot, they can explicitly add +kvm_mmu to the QEMU command-line. Using 2.6.33 and higher, it is not possible to enable kvm_mmu explicitly anymore. 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 Cc: libvir-list@redhat.com Cc: Jiri Denemark jdene...@redhat.com Changes v2: - Coding style fix - Removed redundant comments above machine init functions Changes v3: - Eliminate per-machine-type compatibility code --- target-i386/cpu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 40400ac..b09b625 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -159,7 +159,6 @@ int enforce_cpuid = 0; #if defined(CONFIG_KVM) static uint32_t kvm_default_features = (1 KVM_FEATURE_CLOCKSOURCE) | (1 KVM_FEATURE_NOP_IO_DELAY) | -(1 KVM_FEATURE_MMU_OP) | (1 KVM_FEATURE_CLOCKSOURCE2) | (1 KVM_FEATURE_ASYNC_PF) | (1 KVM_FEATURE_STEAL_TIME) | -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure we always setup a private mount namespace for LXC controller
On 01/07/2013 11:16 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The code for setting up a private /dev/pts for the containers is also responsible for making the LXC controller have a private mount namespace. Unfortunately the /dev/pts code is not run if launching a container without a custom root. This causes the LXC FUSE mount to leak into the host FS. --- src/lxc/lxc_controller.c | 38 ++ 1 file changed, 26 insertions(+), 12 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 1/7] kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code
Any KVM-specific code that use these constants must check if kvm_enabled() is true before using them. 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 --- include/sysemu/kvm.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 3db19ff..15f9658 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -21,6 +21,20 @@ #ifdef CONFIG_KVM #include linux/kvm.h #include linux/kvm_para.h +#else +/* These constants must never be used at runtime if kvm_enabled() is false. + * They exist so we don't need #ifdefs around KVM-specific code that already + * checks kvm_enabled() properly. + */ +#define KVM_CPUID_SIGNATURE 0 +#define KVM_CPUID_FEATURES 0 +#define KVM_FEATURE_CLOCKSOURCE 0 +#define KVM_FEATURE_NOP_IO_DELAY 0 +#define KVM_FEATURE_MMU_OP 0 +#define KVM_FEATURE_CLOCKSOURCE2 0 +#define KVM_FEATURE_ASYNC_PF 0 +#define KVM_FEATURE_STEAL_TIME 0 +#define KVM_FEATURE_PV_EOI 0 #endif extern int kvm_allowed; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 2/7] target-i386: Don't set any KVM flag by default if KVM is disabled
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.
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,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix virLXCPrepareHostDevices method
On 01/07/2013 11:17 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The virLXCPrepareHostDevices method was returning success even when it reported an error, and failed to handle several host device types --- src/lxc/lxc_hostdev.c | 16 +++- 1 file changed, 15 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 qom-cpu 6/7] target-i386/cpu.c: Add feature name array for ext4_features
Feature names were taken from the X86_FEATURE_* constants in the Linux
kernel code.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
Cc: Gleb Natapov g...@redhat.com
---
target-i386/cpu.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4b3ee63..a54c464 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -95,6 +95,17 @@ static const char *ext3_feature_name[] = {
NULL, NULL, NULL, NULL,
};
+static const char *ext4_feature_name[] = {
+NULL, NULL, xstore,xstore-en,
+NULL, NULL, xcrypt,xcrypt-en,
+ace2, ace2-en, phe, phe-en,
+pmm, pmm-en, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+};
+
static const char *kvm_feature_name[] = {
kvmclock, kvm_nopiodelay, kvm_mmu, kvmclock,
kvm_asyncpf, kvm_steal_time, kvm_pv_eoi, NULL,
@@ -147,6 +158,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.feat_names = ext3_feature_name,
.cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
},
+[FEAT_C000_0001_EDX] = {
+.feat_names = ext4_feature_name,
+.cpuid_eax = 0xC001, .cpuid_reg = R_EDX,
+},
[FEAT_KVM] = {
.feat_names = kvm_feature_name,
.cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
@@ -1412,6 +1427,7 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-ext_features |= plus_features[FEAT_1_ECX];
x86_cpu_def-ext2_features |= plus_features[FEAT_8000_0001_EDX];
x86_cpu_def-ext3_features |= plus_features[FEAT_8000_0001_ECX];
+x86_cpu_def-ext4_features |= plus_features[FEAT_C000_0001_EDX];
x86_cpu_def-kvm_features |= plus_features[FEAT_KVM];
x86_cpu_def-svm_features |= plus_features[FEAT_SVM];
x86_cpu_def-cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
@@ -1419,6 +1435,7 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
x86_cpu_def-ext_features = ~minus_features[FEAT_1_ECX];
x86_cpu_def-ext2_features = ~minus_features[FEAT_8000_0001_EDX];
x86_cpu_def-ext3_features = ~minus_features[FEAT_8000_0001_ECX];
+x86_cpu_def-ext4_features = ~minus_features[FEAT_C000_0001_EDX];
x86_cpu_def-kvm_features = ~minus_features[FEAT_KVM];
x86_cpu_def-svm_features = ~minus_features[FEAT_SVM];
x86_cpu_def-cpuid_7_0_ebx_features = ~minus_features[FEAT_7_0_EBX];
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] xen: Avoid possible NULL dereference
On Mon, Jan 07, 2013 at 12:09:29PM -0500, John Ferlan wrote:
Change calling sequence to only call xenUnifiedDomainSetVcpusFlags() when
'dom' is not NULL. Use the GET_PRIVATE() macro to reference privateData.
Just return -1 if dom is NULL.
---
src/xen/xen_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 2ef2c57..559025e 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1253,17 +1253,17 @@ static int
xenUnifiedDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
{
unsigned int flags = VIR_DOMAIN_VCPU_LIVE;
-xenUnifiedPrivatePtr priv;
/* Per the documented API, it is hypervisor-dependent whether this
* affects just _LIVE or _LIVE|_CONFIG; in xen's case, that
* depends on xendConfigVersion. */
if (dom) {
-priv = dom-conn-privateData;
+GET_PRIVATE(dom-conn);
if (priv-xendConfigVersion = XEND_CONFIG_VERSION_3_0_4)
flags |= VIR_DOMAIN_VCPU_CONFIG;
+return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
}
-return xenUnifiedDomainSetVcpusFlags(dom, nvcpus, flags);
+return -1;
}
static int
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
Re: [libvirt] [PATCH 3/6] remote: Avoid calling virAuthConfigLookup() if 'credname' is NULL.
On Mon, Jan 07, 2013 at 12:09:31PM -0500, John Ferlan wrote: --- src/remote/remote_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c078cb5..65fcc5b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3738,7 +3738,8 @@ static int remoteAuthFillFromConfig(virConnectPtr conn, break; } -if (virAuthConfigLookup(state-config, +if (credname +virAuthConfigLookup(state-config, libvirt, VIR_URI_SERVER(conn-uri), credname, 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
Re: [libvirt] [PATCH 2/6] vmware: Avoid NULL dereference for 'caps'
On Mon, Jan 07, 2013 at 12:09:30PM -0500, John Ferlan wrote: When virCapabilitiesNew() fails, caps will be NULL resulting in possible core when deref'd in cpuDataFree() call. --- src/vmware/vmware_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index b514636..fd9c473 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -127,7 +127,8 @@ vmwareCapsInit(void) cleanup: virCPUDefFree(cpu); -cpuDataFree(caps-host.arch, data); +if (caps) +cpuDataFree(caps-host.arch, data); return caps; 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
Re: [libvirt] [PATCH 4/6] lxc: Check for non NULL usb prior to calling usbFreeDevice()
On Mon, Jan 07, 2013 at 12:09:32PM -0500, John Ferlan wrote: --- src/lxc/lxc_driver.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8050ce6..d4b4989 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3468,7 +3468,8 @@ cleanup: if (ret 0 created) unlink(dstfile); -usbFreeDevice(usb); +if (usb) +usbFreeDevice(usb); virCgroupFree(group); VIR_FREE(src); VIR_FREE(dstfile); @@ -4022,7 +4023,8 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, ret = 0; cleanup: -usbFreeDevice(usb); +if (usb) +usbFreeDevice(usb); VIR_FREE(dst); virCgroupFree(group); return ret; NACK to this one. Can you change the 'usbFreeDevice' method to contain a line if (!dev) return so that it matches our preferred coding style of allowing NULLS to 'free()' like methods. Also then add that method to the variable 'useless_free_options' in cfg.mk Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] cpu: Avoid NULL dereference
On Mon, Jan 07, 2013 at 12:09:34PM -0500, John Ferlan wrote: Don't dereference 'model' in PowerPCBaseline when there's no outputModel --- src/cpu/cpu_powerpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 8bef627..5e1a7b9 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -605,7 +605,8 @@ PowerPCBaseline(virCPUDefPtr *cpus, goto error; } -base_model-data-ppc.pvr = model-data-ppc.pvr; +if (outputModel) +base_model-data-ppc.pvr = model-data-ppc.pvr; if (PowerPCDecode(cpu, base_model-data, models, nmodels, NULL) 0) goto error; 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
Re: [libvirt] [PATCH] build: Add libxenctrl to LIBXL_LIBS
On 01/07/2013 11:01 AM, Jim Fehlig wrote: Eric Blake wrote: On 01/07/2013 10:40 AM, Jim Fehlig wrote: Commit dfa1e1dd removed libxenctrl from LIBXL_LIBS, but the libxl driver uses a symbol from this library. Explicitly link with libxenctrl instead of relying on the build system to support implicit DSO linking. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ab08f17..20108c2 100644 --- a/configure.ac +++ b/configure.ac @@ -733,7 +733,7 @@ if test $with_libxl != no ; then LIBS=$LIBS $LIBXL_LIBS AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [ with_libxl=yes -LIBXL_LIBS=$LIBXL_LIBS -lxenlight +LIBXL_LIBS=$LIBXL_LIBS -lxenlight -lxenctrl ACK. Thanks, pushed. As long as I'm backporting libxl stuff to v0.10.2-maint, I did this one too. -- 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 5/6] lxc: Avoid possible NULL dereference on *root prior to opendir().
On Mon, Jan 07, 2013 at 12:09:33PM -0500, John Ferlan wrote:
If running on older Linux without mounted cgroups then its possible that
*root would be NULL.
---
src/lxc/lxc_container.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index d3a2968..d234426 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1762,6 +1762,12 @@ static int lxcContainerIdentifyCGroups(struct
lxcContainerCGroup **mountsret,
VIR_DEBUG(Grabbed '%s', mntent.mnt_dir);
}
+if (!*root) {
+VIR_DEBUG(No mounted cgroups found);
+ret = 0;
+goto cleanup;
+}
+
VIR_DEBUG(Checking for symlinks in %s, *root);
if (!(dh = opendir(*root))) {
virReportSystemError(errno,
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] libvirt-1.0.1 make check failure
Hello, I just attempted compilation and testing of libvirt-1.0.1 on an RHEL-6.3 x86_64 system and received one failure noted here: ... TEST: qemumonitorjsontest !5 FAIL FAIL: qemumonitorjsontest ... === 1 of 68 tests failed (1 test was not run) Please report to libvir-list@redhat.com === The whole reason that I am even attempting this is because I have not be able to successfully craft a chroot LXC container using the older 0.9.10-21.el6_3.6 versions that I can get through yum. When attempting to start that container I get: # virsh start base error: Failed to start domain base error: internal error guest failed to start: 2013-01-07 18:48:31.382+: 2397: info : libvirt version: 0.9.10, package: 21.el6_3.6 (Red Hat, Inc. http://bugzilla.redhat.com/bugzilla, 2012-11-12-11:08:41, x86-008.build.bos.redhat.com) 2013-01-07 18:48:31.382+: 2397: error : lxcControllerRun:1484 : Failed to query file context on /lxc/base_rhel-6.3-i686: No data available The /lxc/base_rhel-6.3-i686 directory contains a basic installation of an i686 choot environment which I can interact fine with using chroot. I have not been able to figure out what error : lxcControllerRun:1484 : Failed to query file context on /lxc/base_rhel-6.3-i686: No data available is trying to tell me and was hoping that maybe a newer version of libvirt may be more friendly (though this is more likely coming from lxc code). Does anyone have suggestions? Thanks in advance. - Marc -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add RESUME event listener to qemu monitor.
Perform all the appropriate plumbing.
When qemu/KVM VMs are paused manually through a monitor not-owned by libvirt,
libvirt will think of them as paused event after they are resumed and
effectively running. With this patch the discrepancy goes away.
This is meant to address bug 892791.
Signed-off-by: Andres Lagar-Cavilla and...@lagarcavilla.org
---
src/qemu/qemu_monitor.c | 10
src/qemu/qemu_monitor.h |3 +++
src/qemu/qemu_monitor_json.c |7 ++
src/qemu/qemu_process.c | 56 ++
4 files changed, 76 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 99642b6..cb5a3e2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1022,6 +1022,16 @@ int qemuMonitorEmitStop(qemuMonitorPtr mon)
}
+int qemuMonitorEmitResume(qemuMonitorPtr mon)
+{
+int ret = -1;
+VIR_DEBUG(mon=%p, mon);
+
+QEMU_MONITOR_CALLBACK(mon, ret, domainResume, mon-vm);
+return ret;
+}
+
+
int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset)
{
int ret = -1;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d7faa90..00ce813 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -97,6 +97,8 @@ struct _qemuMonitorCallbacks {
virDomainObjPtr vm);
int (*domainStop)(qemuMonitorPtr mon,
virDomainObjPtr vm);
+int (*domainResume)(qemuMonitorPtr mon,
+virDomainObjPtr vm);
int (*domainRTCChange)(qemuMonitorPtr mon,
virDomainObjPtr vm,
long long offset);
@@ -187,6 +189,7 @@ int qemuMonitorEmitShutdown(qemuMonitorPtr mon);
int qemuMonitorEmitReset(qemuMonitorPtr mon);
int qemuMonitorEmitPowerdown(qemuMonitorPtr mon);
int qemuMonitorEmitStop(qemuMonitorPtr mon);
+int qemuMonitorEmitResume(qemuMonitorPtr mon);
int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset);
int qemuMonitorEmitWatchdog(qemuMonitorPtr mon, int action);
int qemuMonitorEmitIOError(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2d2d254..de5f115 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -55,6 +55,7 @@ static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr mon,
virJSONValuePtr da
static void qemuMonitorJSONHandleReset(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandlePowerdown(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandleStop(qemuMonitorPtr mon, virJSONValuePtr
data);
+static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandleWatchdog(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandleIOError(qemuMonitorPtr mon, virJSONValuePtr
data);
@@ -87,6 +88,7 @@ static qemuEventHandler eventHandlers[] = {
{ DEVICE_TRAY_MOVED, qemuMonitorJSONHandleTrayChange, },
{ POWERDOWN, qemuMonitorJSONHandlePowerdown, },
{ RESET, qemuMonitorJSONHandleReset, },
+{ RESUME, qemuMonitorJSONHandleResume, },
{ RTC_CHANGE, qemuMonitorJSONHandleRTCChange, },
{ SHUTDOWN, qemuMonitorJSONHandleShutdown, },
{ SPICE_CONNECTED, qemuMonitorJSONHandleSPICEConnect, },
@@ -589,6 +591,11 @@ static void qemuMonitorJSONHandleStop(qemuMonitorPtr mon,
virJSONValuePtr data A
qemuMonitorEmitStop(mon);
}
+static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr
data ATTRIBUTE_UNUSED)
+{
+qemuMonitorEmitResume(mon);
+}
+
static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr
data)
{
long long offset = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f5fc1b3..938c17e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -736,6 +736,61 @@ unlock:
static int
+qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+virDomainObjPtr vm)
+{
+virQEMUDriverPtr driver = qemu_driver;
+virDomainEventPtr event = NULL;
+
+virDomainObjLock(vm);
+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+if (priv-gotShutdown) {
+VIR_DEBUG(Ignoring RESUME event after SHUTDOWN);
+goto unlock;
+}
+
+VIR_DEBUG(Transitioned guest %s out of paused into resumed state,
+ vm-def-name);
+
+virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
+ VIR_DOMAIN_RUNNING_UNPAUSED);
+event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
+
+VIR_DEBUG(Using lock state '%s' on resume event,
