[libvirt] [PATCH] add the check of memory size in xenXMDriver for setmem of Xen domain
Hi, I make the patch. This patch adds the check of memory size in xenXMDriver, to handle the xenStoreDriver error when the set value by setmem(setmaxmem) is 4096-65535 definitely. Thanks, Shigeki Sakamoto. Index: src/xm_internal.c === RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.95 diff -u -p -r1.95 xm_internal.c --- src/xm_internal.c 24 Oct 2008 11:20:08 - 1.95 +++ src/xm_internal.c 31 Oct 2008 05:52:27 - @@ -1273,6 +1273,8 @@ int xenXMDomainSetMemory(virDomainPtr do return (-1); if (domain-id != -1) return (-1); +if (memory 1024 * MIN_XEN_GUEST_SIZE); +return (-1); if (!(filename = virHashLookup(nameConfigMap, domain-name))) return (-1); At first, libvirt.c should not check VMM-arch specific value. In this meaning, xs_internal.c do the right thing. The problem is error handling on libvirt.c(Xen Store error is not handled properly). Do you investigate it? Thanks Atsushi SAKAI S.Sakamoto [EMAIL PROTECTED] wrote: Hi, I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c? In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated. # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory # echo $? 0 As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c. Thanks, Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add the check of memory size in xenXMDriver forsetmem of Xen domain
Hi, Daniel, This patch itself is OK. But I also think MIN_XEN_GUEST_SIZE should be defined in xen_unified.h not src/internal.h. Since this variable is Xen specific. How do you think? Thanks Atsushi SAKAI S.Sakamoto [EMAIL PROTECTED] wrote: Hi, I make the patch. This patch adds the check of memory size in xenXMDriver, to handle the xenStoreDriver error when the set value by setmem(setmaxmem) is 4096-65535 definitely. Thanks, Shigeki Sakamoto. Index: src/xm_internal.c === RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.95 diff -u -p -r1.95 xm_internal.c --- src/xm_internal.c 24 Oct 2008 11:20:08 - 1.95 +++ src/xm_internal.c 31 Oct 2008 05:52:27 - @@ -1273,6 +1273,8 @@ int xenXMDomainSetMemory(virDomainPtr do return (-1); if (domain-id != -1) return (-1); +if (memory 1024 * MIN_XEN_GUEST_SIZE); +return (-1); if (!(filename = virHashLookup(nameConfigMap, domain-name))) return (-1); At first, libvirt.c should not check VMM-arch specific value. In this meaning, xs_internal.c do the right thing. The problem is error handling on libvirt.c(Xen Store error is not handled properly). Do you investigate it? Thanks Atsushi SAKAI S.Sakamoto [EMAIL PROTECTED] wrote: Hi, I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c? In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated. # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory # echo $? 0 As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c. Thanks, Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: don't print uninitialized integer in diagnostic
On Thu, Oct 30, 2008 at 05:26:21PM +0100, Jim Meyering wrote: In adding a test of the vcpu cpuset parsing code (another patch coming separately), I noticed a bogus diagnostic: virsh --connect test:///default dumpxml 1 xml sed s/vcpu/vcpu cpuset='aaa'/ xml xml-invalid ./virsh --connect test:///default define xml-invalid 21 |head -1 libvir: Domain Config error : failed Xen syscall topology cpuset syntax error -2027441560 With the patch below, I get this output instead: (i.e., same, but without the trailing negative number) $ ./virsh --connect test:///default define xml-invalid 21 |head -1 libvir: Domain Config error : failed Xen syscall topology cpuset syntax error Ouch, sure, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] * tests/cpuset: New script. Test for today's fix.
On Thu, Oct 30, 2008 at 05:43:18PM +0100, Jim Meyering wrote: From c0beb0ed6dd3d392b11161c565d7dfd52ed2aceb Mon Sep 17 00:00:00 2001 From: Jim Meyering [EMAIL PROTECTED] Date: Mon, 2 Jun 2008 11:54:34 +0200 Subject: [PATCH 1/2] * tests/cpuset: New script. Test for today's fix. * tests/Makefile.am (test_scripts): Add cpuset. yes please :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add the check of memory size in xenXMDriver forsetmem of Xen domain
On Fri, Oct 31, 2008 at 05:34:24PM +0900, Atsushi SAKAI wrote: Hi, Daniel, This patch itself is OK. But I also think MIN_XEN_GUEST_SIZE should be defined in xen_unified.h not src/internal.h. Since this variable is Xen specific. Yes, I believe one of my patches from yesterday moves this. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Domain Events Python Bindings (Round 2)
On Thu, Oct 30, 2008 at 05:03:28PM -0400, Ben Guthro wrote: Attached are the python bindings for domain events as previously submitted: https://www.redhat.com/archives/libvir-list/2008-October/msg00707.html https://www.redhat.com/archives/libvir-list/2008-October/msg00668.html I have resolved most of the issues Daniel V. commented on I also addressed a number of ref counting problems. Okidoc, I think that's good to go, so applied and commited :-) One of the things I need to check/fix is that the examples from examples/domain-events are also added to the RPMs in the docs section, i will check this, there is a bit of nastyness to avoid problem with multilib if not careful. thanks a lot ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]: Update ruby-libvirt migrate binding to use rb_scan_args
Chris Lalancette wrote: It's really not a good idea to hand parse variable number of args to a ruby binding, like I implemented for the migrate method. Convert this to use rb_scan_args, which is the proper way to do it, and matches all of the other variable argument implementations in the binding. Committed. Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
RE: [libvirt] Domain Events Python Bindings (Round 2)
Hmmm... I also realized that I never added the example to the top level Makefile.am EXTRA_DIST. I imagine that would need to be resolved to address adding it to the RPM Would you like me to take a look at this, or will this be something you'll resolve as part of the RPM multlib cleanup? Ben -Original Message- From: Daniel Veillard [mailto:[EMAIL PROTECTED] Sent: Fri 10/31/2008 6:16 AM To: Ben Guthro Cc: libvir-list@redhat.com Subject: Re: [libvirt] Domain Events Python Bindings (Round 2) On Thu, Oct 30, 2008 at 05:03:28PM -0400, Ben Guthro wrote: Attached are the python bindings for domain events as previously submitted: https://www.redhat.com/archives/libvir-list/2008-October/msg00707.html https://www.redhat.com/archives/libvir-list/2008-October/msg00668.html I have resolved most of the issues Daniel V. commented on I also addressed a number of ref counting problems. Okidoc, I think that's good to go, so applied and commited :-) One of the things I need to check/fix is that the examples from examples/domain-events are also added to the RPMs in the docs section, i will check this, there is a bit of nastyness to avoid problem with multilib if not careful. thanks a lot ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH]: Allow arbitrary paths to virStorageVolLookupByPath
In ovirt, we have to scan iSCSI LUN's for LVM storage when they are first added
to the database. To do this, we do roughly the following:
iscsi_pool = libvirt.define_and_start_iscsi_pool(/dev/disk/by-id)
iscsi_pool.add_luns_to_db
logical = conn.discover_storage_pool_sources(logical)
for each logical_volume_group in logical:
for each device in logical_volume_group:
iscsi_pool.lookup_vol_by_path(device.path)
And then we use that information to associate an iSCSI LUN with this volume
group. The problem is that there is an mis-match between how we define the
iscsi pool (with /dev/disk/by-id devices), and what data the
discover_storage_pool_sources() returns (/dev devices), so we can't easily
associate the two.
The following patch implements stable path naming when the
virStorageVolLookupByPath method is called. Basically, it tries to convert
whatever path it is given (say /dev/sdc) into the form currently used by the
Pool (say /dev/disk/by-id). It then goes and looks up the form in the pool, and
returns the storageVolume object as appropriate. Note that it only tries to do
this for the Pool types where it makes sense, namely iSCSI and disk; all other
pool types stay exactly the same.
With this in place, we can solve the mis-match in the above code, and LVM
scanning seems to work in ovirt.
Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
Index: src/storage_backend.h
===
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.9
diff -u -r1.9 storage_backend.h
--- a/src/storage_backend.h 23 Oct 2008 11:32:22 - 1.9
+++ b/src/storage_backend.h 31 Oct 2008 10:09:29 -
@@ -50,6 +50,7 @@
VIR_STORAGE_BACKEND_POOL_SOURCE_DIR = (12),
VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER = (13),
VIR_STORAGE_BACKEND_POOL_SOURCE_NAME= (14),
+VIR_STORAGE_BACKEND_POOL_STABLE_PATH= (15),
};
enum partTableType {
Index: src/storage_backend_disk.c
===
RCS file: /data/cvs/libvirt/src/storage_backend_disk.c,v
retrieving revision 1.16
diff -u -r1.16 storage_backend_disk.c
--- a/src/storage_backend_disk.c 23 Oct 2008 11:32:22 - 1.16
+++ b/src/storage_backend_disk.c 31 Oct 2008 10:09:29 -
@@ -447,7 +447,8 @@
.deleteVol = virStorageBackendDiskDeleteVol,
.poolOptions = {
-.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE),
+.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE|
+ VIR_STORAGE_BACKEND_POOL_STABLE_PATH),
.defaultFormat = VIR_STORAGE_POOL_DISK_UNKNOWN,
.formatFromString = virStorageBackendPartTableTypeFromString,
.formatToString = virStorageBackendPartTableTypeToString,
Index: src/storage_backend_iscsi.c
===
RCS file: /data/cvs/libvirt/src/storage_backend_iscsi.c,v
retrieving revision 1.15
diff -u -r1.15 storage_backend_iscsi.c
--- a/src/storage_backend_iscsi.c 16 Oct 2008 15:06:04 - 1.15
+++ b/src/storage_backend_iscsi.c 31 Oct 2008 10:09:29 -
@@ -645,7 +645,8 @@
.poolOptions = {
.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_HOST |
- VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE)
+ VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE |
+ VIR_STORAGE_BACKEND_POOL_STABLE_PATH)
},
.volType = VIR_STORAGE_VOL_BLOCK,
Index: src/storage_driver.c
===
RCS file: /data/cvs/libvirt/src/storage_driver.c,v
retrieving revision 1.13
diff -u -r1.13 storage_driver.c
--- a/src/storage_driver.c 21 Oct 2008 17:15:53 - 1.13
+++ b/src/storage_driver.c 31 Oct 2008 10:09:29 -
@@ -963,11 +963,25 @@
virStorageDriverStatePtr driver =
(virStorageDriverStatePtr)conn-storagePrivateData;
unsigned int i;
+char *stable_path;
for (i = 0 ; i driver-pools.count ; i++) {
if (virStoragePoolObjIsActive(driver-pools.objs[i])) {
-virStorageVolDefPtr vol =
-virStorageVolDefFindByPath(driver-pools.objs[i], path);
+virStorageVolDefPtr vol;
+virStorageBackendPoolOptionsPtr options;
+
+options = virStorageBackendPoolOptionsForType(driver-pools.objs[i]-def-type);
+if (options == NULL)
+continue;
+
+if (options-flags VIR_STORAGE_BACKEND_POOL_STABLE_PATH)
+stable_path = virStorageBackendStablePath(conn, driver-pools.objs[i], (char *)path);
+else
+stable_path = (char *)path;
+
+vol = virStorageVolDefFindByPath(driver-pools.objs[i], stable_path);
+if (stable_path != path)
+VIR_FREE(stable_path);
if (vol)
return virGetStorageVol(conn,
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]: Allow arbitrary paths to virStorageVolLookupByPath
Daniel P. Berrange wrote:
Personally, I think those are bad semantics for virStorageBackendStablePath;
assuming it succeeds, you should always be able to know that you have a copy,
regardless of whether the copy is the same as the original. Should I change
virStorageBackendStablePath to those semantics, in which case your below code
would then be correct?
Yes, I think that's worth doing - will also avoid the cast in the input
arg there
OK, updated patch attached; virStorageBackendStablePath now always returns a
copy of the given string, so it's always safe to unconditionally VIR_FREE it. I
fixed up storage_backend_iscsi and storage_backend_disk to reflect this change.
I also re-worked the code as you suggested, and added a bit more error
checking.
Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
Index: src/storage_backend.c
===
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.24
diff -u -r1.24 storage_backend.c
--- src/storage_backend.c 28 Oct 2008 17:48:06 - 1.24
+++ src/storage_backend.c 31 Oct 2008 11:56:33 -
@@ -357,7 +357,7 @@
char *
virStorageBackendStablePath(virConnectPtr conn,
virStoragePoolObjPtr pool,
-char *devpath)
+const char *devpath)
{
DIR *dh;
struct dirent *dent;
@@ -366,7 +366,7 @@
if (pool-def-target.path == NULL ||
STREQ(pool-def-target.path, /dev) ||
STREQ(pool-def-target.path, /dev/))
-return devpath;
+return strdup(devpath);
/* The pool is pointing somewhere like /dev/disk/by-path
* or /dev/disk/by-id, so we need to check all symlinks in
@@ -410,7 +410,7 @@
/* Couldn't find any matching stable link so give back
* the original non-stable dev path
*/
-return devpath;
+return strdup(devpath);
}
Index: src/storage_backend.h
===
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.9
diff -u -r1.9 storage_backend.h
--- src/storage_backend.h 23 Oct 2008 11:32:22 - 1.9
+++ src/storage_backend.h 31 Oct 2008 11:56:34 -
@@ -50,6 +50,7 @@
VIR_STORAGE_BACKEND_POOL_SOURCE_DIR = (12),
VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER = (13),
VIR_STORAGE_BACKEND_POOL_SOURCE_NAME= (14),
+VIR_STORAGE_BACKEND_POOL_STABLE_PATH= (15),
};
enum partTableType {
@@ -138,7 +139,7 @@
char *virStorageBackendStablePath(virConnectPtr conn,
virStoragePoolObjPtr pool,
- char *devpath);
+ const char *devpath);
typedef int (*virStorageBackendListVolRegexFunc)(virConnectPtr conn,
virStoragePoolObjPtr pool,
Index: src/storage_backend_disk.c
===
RCS file: /data/cvs/libvirt/src/storage_backend_disk.c,v
retrieving revision 1.16
diff -u -r1.16 storage_backend_disk.c
--- src/storage_backend_disk.c 23 Oct 2008 11:32:22 - 1.16
+++ src/storage_backend_disk.c 31 Oct 2008 11:56:34 -
@@ -109,8 +109,7 @@
devpath)) == NULL)
return -1;
-if (devpath != vol-target.path)
-VIR_FREE(devpath);
+VIR_FREE(devpath);
}
if (vol-key == NULL) {
@@ -447,7 +446,8 @@
.deleteVol = virStorageBackendDiskDeleteVol,
.poolOptions = {
-.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE),
+.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE|
+ VIR_STORAGE_BACKEND_POOL_STABLE_PATH),
.defaultFormat = VIR_STORAGE_POOL_DISK_UNKNOWN,
.formatFromString = virStorageBackendPartTableTypeFromString,
.formatToString = virStorageBackendPartTableTypeToString,
Index: src/storage_backend_iscsi.c
===
RCS file: /data/cvs/libvirt/src/storage_backend_iscsi.c,v
retrieving revision 1.15
diff -u -r1.15 storage_backend_iscsi.c
--- src/storage_backend_iscsi.c 16 Oct 2008 15:06:04 - 1.15
+++ src/storage_backend_iscsi.c 31 Oct 2008 11:56:34 -
@@ -219,8 +219,7 @@
devpath)) == NULL)
goto cleanup;
-if (devpath != vol-target.path)
-VIR_FREE(devpath);
+VIR_FREE(devpath);
if (virStorageBackendUpdateVolInfoFD(conn, vol, fd, 1) 0)
goto cleanup;
@@ -645,7 +644,8 @@
.poolOptions = {
.flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_HOST |
- VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE)
+ VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE |
+ VIR_STORAGE_BACKEND_POOL_STABLE_PATH)
},
.volType = VIR_STORAGE_VOL_BLOCK,
Index: src/storage_driver.c
Re: [libvirt] [PATCH]: Allow arbitrary paths to virStorageVolLookupByPath
On Fri, Oct 31, 2008 at 12:26:17PM +0100, Chris Lalancette wrote:
Daniel P. Berrange wrote:
On Fri, Oct 31, 2008 at 12:04:34PM +0100, Chris Lalancette wrote:
Index: src/storage_driver.c
===
RCS file: /data/cvs/libvirt/src/storage_driver.c,v
retrieving revision 1.13
diff -u -r1.13 storage_driver.c
--- a/src/storage_driver.c 21 Oct 2008 17:15:53 - 1.13
+++ b/src/storage_driver.c 31 Oct 2008 10:09:29 -
@@ -963,11 +963,25 @@
virStorageDriverStatePtr driver =
(virStorageDriverStatePtr)conn-storagePrivateData;
unsigned int i;
+char *stable_path;
for (i = 0 ; i driver-pools.count ; i++) {
if (virStoragePoolObjIsActive(driver-pools.objs[i])) {
-virStorageVolDefPtr vol =
-virStorageVolDefFindByPath(driver-pools.objs[i], path);
+virStorageVolDefPtr vol;
+virStorageBackendPoolOptionsPtr options;
+
+options =
virStorageBackendPoolOptionsForType(driver-pools.objs[i]-def-type);
+if (options == NULL)
+continue;
+
+if (options-flags VIR_STORAGE_BACKEND_POOL_STABLE_PATH)
+stable_path = virStorageBackendStablePath(conn,
driver-pools.objs[i], (char *)path);
+else
+stable_path = (char *)path;
+
+vol = virStorageVolDefFindByPath(driver-pools.objs[i],
stable_path);
+if (stable_path != path)
+VIR_FREE(stable_path);
This VIR_FREE check is slightly evil, how about something more like
Well, I originally stole this pointer comparison from storage_backend_iscsi.c
which does the same thing. While I agree that the code below is more clear,
it
actually has a subtle bug; if you pass in, say /dev/sdc, and the pool target
path is of type /dev, then virStorageBackendStablePath() will return the
original pointer, not a copy. So in the below code, you would actually end up
freeing path, not a copy of path.
Personally, I think those are bad semantics for virStorageBackendStablePath;
assuming it succeeds, you should always be able to know that you have a copy,
regardless of whether the copy is the same as the original. Should I change
virStorageBackendStablePath to those semantics, in which case your below code
would then be correct?
Yes, I think that's worth doing - will also avoid the cast in the input
arg there
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 8/11: Make better use of linker scripts
On Thu, Oct 30, 2008 at 04:56:30PM +, Daniel P. Berrange wrote:
On Thu, Oct 30, 2008 at 01:41:14PM +, Daniel P. Berrange wrote:
The libvirt.so file currently whitelists all the symbols in our public
API. libvirtd and virsh, however, also want to use a bunch of our so
called 'private' symbols which aren't in the public API. For saferead
and safewrite() we dealt with this by compiling the code twice with
some nasty macro tricks to give the function a different name to avoid
dup symbols when statically linking. For the other APIs, we prefixed
them with __ and then just added them to the exports.
Neither option is very good because they both impose a burden on the
source code - needing to append __ everywhere and write crazy macros.
Each time we want to export another symbol, we have to make lots of
manual changes to add __. This was OK if it was just a handful of
symbols, but we're now upto 30 odd, and its not scaling. When we
make drivers modular we'll be adding 100's more symbols.
As an aside, we use ELF library versioning when linking, via the
libtool -version-info flag. Since we maintain ABI compat going
forwards though, the version in the eventual .so is always going
to be 'libvirt.so.0'. This is sub-optimal because you may build
a binary that requires 'virDomainBlockPeek' which appeared in
libvirt 0.4.2, but the ELF version cannot validate this. There is
nothing stopping your application launching against libvirt 0.3.0
and then aborting with a linker failure sometime while it is
running when it eventually referneces the non-existant symbol.
Likewise RPM automatic versioning hooks off the ELF version too,
so that's not preventing you installing too old a libvirt for
your app's needs.
A far better solution to all these problems has been sitting in
front of us the whole time. Namely we need to make full use of
the linkers symbol version scripts.
Our current libvirt_sym.version script is defining an unversioned
set of symbols. This only enforces what's exported, and can do
nothing about version compatability. So instead we need to switch
to a fully versioned script, where every symbol we export is tagged
with the libvirt version number at which it was introduced.
Taking the virDomainBlockPeek example again, this appeared in the
libvirt 0.4.2 release, so we'd add a section to the linker script
LIBVIRT_0.4.2 {
global:
virDomainBlockPeek;
virDomainMemoryPeek;
} LIBVIRT_0.4.1;
Then 0.4.5 added in storage pool discovery so you get another
section
LIBVIRT_0.4.5 {
global:
virConnectFindStoragePoolSources;
} LIBVIRT_0.4.2;
And so on for every release.
The resulting libvirt.so file will thus gain metadata listing all
the ABI versions for all symbols it includes.
That deals with public APIs. Now we also want to export some internal
APIs whose semantics/syntax may arbitrarily change between releases.
Thus they should not be included in any of the formal public version
sections.
Instead they should be in a seperate versioned section, whose version
changes on every release. This will ensure that if libvirtd or virsh
link to some internal symbols, they are guareteened to only run
against the exactly same libvirt.so they were linked to. This will
avoid some nasty bugs our users have hit where they installed a custom
libvirt version on their OS which already had another version installed,
and then libvirtd crashed/behave wierdly/etc
To do this we rename libvirt_sym.version to libvirt_sym.version.in
and add a section called
[EMAIL PROTECTED]@ {
global:
/* libvirt_internal.h */
debugFlag;
virStateInitialize;
virStateCleanup;
virStateReload;
virStateActive;
more private symbols...
}
The @VERSION@ gets subsituted by the version number of the libvirt
release by configure.
Do a build with this all active and look at the resulting libvirt.so
using objdump (or eu-readelf).
# objdump -p src/.libs/libvirt.so
Version definitions:
1 0x01 0x0e5a1d10 libvirt.so.0
2 0x00 0x0af6bd33 LIBVIRT_0.0.3
3 0x00 0x0af6bd35 LIBVIRT_0.0.5
LIBVIRT_0.0.3
4 0x00 0x0af6be30 LIBVIRT_0.1.0
LIBVIRT_0.0.5
5 0x00 0x0af6be31 LIBVIRT_0.1.1
LIBVIRT_0.1.0
6 0x00 0x0af6be34 LIBVIRT_0.1.4
LIBVIRT_0.1.1
7 0x00 0x0af6be35 LIBVIRT_0.1.5
LIBVIRT_0.1.4
8 0x00 0x0af6be39 LIBVIRT_0.1.9
LIBVIRT_0.1.5
9 0x00 0x0af6bb30 LIBVIRT_0.2.0
LIBVIRT_0.1.9
10 0x00 0x0af6bb31 LIBVIRT_0.2.1
LIBVIRT_0.2.0
11 0x00 0x0af6bb33 LIBVIRT_0.2.3
LIBVIRT_0.2.1
12 0x00 0x0af6bc30 LIBVIRT_0.3.0
LIBVIRT_0.2.3
13 0x00 0x0af6bc32 LIBVIRT_0.3.2
LIBVIRT_0.3.0
14 0x00 0x0af6bc33 LIBVIRT_0.3.3
LIBVIRT_0.3.2
15 0x00 0x0af6b930 LIBVIRT_0.4.0
LIBVIRT_0.3.3
16
RE: [libvirt] Domain Events Python Bindings (Round 2)
I just took a look over the commit here: http://git.et.redhat.com/?p=libvirt.git;a=commit;h=471b765ae0fc3efd721d6dbd3c82539880933c0b It appears that you may have missed adding the new file python/virConnect.py This file is critical to this patch, as it controls proper callback registration, and cleanup coordination with the C code Ben -Original Message- From: Daniel Veillard [mailto:[EMAIL PROTECTED] Sent: Fri 10/31/2008 7:22 AM To: Ben Guthro Cc: libvir-list@redhat.com Subject: Re: [libvirt] Domain Events Python Bindings (Round 2) On Fri, Oct 31, 2008 at 07:04:09AM -0400, Ben Guthro wrote: Hmmm... I also realized that I never added the example to the top level Makefile.am EXTRA_DIST. I imagine that would need to be resolved to address adding it to the RPM Would you like me to take a look at this, or will this be something you'll resolve as part of the RPM multlib cleanup? I will handle this :-) don't worry ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add the check of memory size in xenXMDriver forsetmem of Xen domain
On Fri, Oct 31, 2008 at 05:34:24PM +0900, Atsushi SAKAI wrote: Hi, Daniel, This patch itself is OK. Yes except for the extra ; after the if making it return -1 all the time, but once fixed sure :-) Patch applied and commited [...] +if (memory 1024 * MIN_XEN_GUEST_SIZE); if (memory 1024 * MIN_XEN_GUEST_SIZE) +return (-1); Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 10/11: Build stateful drivers into libvirtd instead of libvirt.so
On Thu, Oct 30, 2008 at 01:42:19PM +, Daniel P. Berrange wrote:
This patches changes the way some of the drivers are built. Specifically
the stateful drivers (QEMU, LXC, Network and Storage) are no longer
compiled into libvirt.so Instead they are linked directly to the libvirt
binary. They could only ever be executed as part of the daemon, and we had
code checks to ensure this, so this just makes it sane at build time.
Hum if you force the QEmu driver in the libvirtd daemon,
hor do you implement qemu:///session ? I think I'm missing something
there...
More importantly this is required for David Lively's host device support.
That support uses HAL, which uses DBus, which is GPL/AFL licensed. As
such we cannot ever link HAL/DBus against libvirt.so without loosing
our LGPL status. With this patch, HAL/DBus bits will only ever be linked
into the libvirtd daemon binary, so there is no LGPL compat problems in
libvirt.so
Yup, understood and agreed. But all we need is force just the
hardware discovery to be linkeable only by the daemon, not everything
has to be linked that way.
The changes this patch does are fairly simple
- qemud/Makefile.am - add the libtool convenience libraries for qemu,
lxc, storage and network drivers to libvirtd
- src/Makefile.am - remove the libtool convenience libraries for
qemu, lxc, storage network drivers from libvirt.la
- src/libvirt.c: Do not invoke the register methods for qemu, lxc,
storage or network drivers in virInitialize()
- qemud/qemud.c: Invoke the register methods for qemu, lxc, storage
and network drivers after first calling virInitialize()
- libvirt_sym.version.in: add all the internal symbols needed to be
exported for qemu, lxc, storage network drivers
The end result here is functionally identical to before, with one
annoying exception. If you passed a 'NULL' uri to virConnectOpen
it would manually probe each driver to find a suitable URI. Since
QEMU, LXC drivers are not in libvirt.so anymore this probing
ceases to work.
Hum, we should find a way to have the do_open(NULL) probe done into
the server located on localhost. Maybe this means adding an extra remote
operation querying the node for a default hypervisor.
LIBVIRT_DEFAULT_URI is still being checked in the library code, so
that default behaviour can still be overriden by the user if this leads
to picking the wrong driver.
I've not figured out best way to deal with this yet. One option is
to not actually move the drivers out of libvirt.so at all. Just do
it for the forthcoming node devices driver. ANother option is to just
have a tiny QEMU/LXC stub driver in libvirt.so solely containing the
probe() code. A further option is to make the remote driver implement
the probe() method, so it actally talks to libvirtd and asks that
for a probed URI.
IMHO the 3rd solution is the one making most sense from an
architecture POV. Maybe we can even get this to work for remote and
be able to ask a remote node if it supports Xen or KVM by default
(or something else ...)
--- a/src/libvirt_sym.version.in Thu Oct 30 10:46:21 2008 +
+++ b/src/libvirt_sym.version.in Thu Oct 30 11:04:27 2008 +
@@ -257,6 +257,19 @@
[EMAIL PROTECTED]@ {
global:
+ /* bridge.h */
+ brAddBridge;
+ brAddInterface;
+ brAddTap;
+ brDeleteBridge;
+ brInit;
+ brSetEnableSTP;
+ brSetForwardDelay;
+ brSetInetAddress;
+ brSetInetNetmask;
+ brSetInterfaceUp;
+ brShutdown;
+
/* buf.h */
virBufferVSprintf;
@@ -264,6 +277,18 @@
virBufferAddChar;
virBufferContentAndReset;
virBufferError;
+
+
+ /* caps.h */
+ virCapabilitiesAddGuest;
+ virCapabilitiesAddGuestDomain;
+ virCapabilitiesAddGuestFeature;
+ virCapabilitiesAddHostNUMACell;
+ virCapabilitiesDefaultGuestEmulator;
+ virCapabilitiesFormatXML;
+ virCapabilitiesFree;
+ virCapabilitiesNew;
+ virCapabilitiesSetMacPrefix;
/* conf.h */
@@ -284,7 +309,62 @@
virGetStorageVol;
+ /* domain_conf.h */
+ virDiskNameToBusDeviceIndex;
+ virDiskNameToIndex;
+ virDomainAssignDef;
+ virDomainConfigFile;
+ virDomainDefDefaultEmulator;
+ virDomainDefFormat;
+ virDomainDefFree;
+ virDomainDefParseFile;
+ virDomainDefParseString;
+ virDomainDeleteConfig;
+ virDomainDeviceDefParse;
+ virDomainDiskBusTypeToString;
+ virDomainDiskDeviceTypeToString;
+ virDomainDiskQSort;
+ virDomainEventCallbackListAdd;
+ virDomainEventCallbackListFree;
+ virDomainEventCallbackListRemove;
+ virDomainFindByID;
+ virDomainFindByName;
+ virDomainFindByUUID;
+ virDomainLoadAllConfigs;
+ virDomainObjListFree;
+ virDomainRemoveInactive;
+ virDomainSaveConfig;
+ virDomainSoundModelTypeToString;
+ virDomainVirtTypeToString;
+
+
+ /*
Re: [libvirt] Domain Events Python Bindings (Round 2)
On Fri, Oct 31, 2008 at 07:44:09AM -0400, Ben Guthro wrote: I just took a look over the commit here: http://git.et.redhat.com/?p=libvirt.git;a=commit;h=471b765ae0fc3efd721d6dbd3c82539880933c0b It appears that you may have missed adding the new file python/virConnect.py This file is critical to this patch, as it controls proper callback registration, and cleanup coordination with the C code Whoops, right ! Pushed too now, sorry and thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
