Re: [libvirt] [PATCHv2 9/7] snapshot: simplify esx snapshot name lookup
2011/10/3 Eric Blake ebl...@redhat.com: No need to request the parent of a snapshot if we aren't going to use it. * src/esx/esx_vi.c (esxVI_GetSnapshotTreeByName): Make parent optional. * src/esx/esx_driver.c (esxDomainSnapshotCreateXML) (esxDomainSnapshotLookupByName, esxDomainRevertToSnapshot) (esxDomainSnapshotDelete): Simplify accordingly. --- src/esx/esx_driver.c | 12 src/esx/esx_vi.c | 7 --- 2 files changed, 8 insertions(+), 11 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/7] snapshot: implement snapshot children listing in esx
2011/10/3 Eric Blake ebl...@redhat.com: It was fairly trivial to return snapshot listing based on a point in the hierarchy, rather than starting at all roots. * src/esx/esx_driver.c (esxDomainSnapshotNumChildren) (esxDomainSnapshotListChildrenNames): New functions. --- src/esx/esx_driver.c | 99 ++ 1 files changed, 99 insertions(+), 0 deletions(-) Tested, works, ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 11/7] snapshot: optimize vbox snapshot name lookup
2011/10/4 Eric Blake ebl...@redhat.com:
Older VBox required grabbing all snapshots, then looking through
them until a name match was found. But when VBox 3.1 introduced
snapshot branching, it also added the ability to lookup a snapshot
by name instead of UUID; exploit this for faster snapshot lookup.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotGet): Newer vbox added
snapshot lookup by name.
---
Caveat - this is written solely by reading VBox documentation, and
I was only able to compile test. I have no idea if this really
works, or if VBox 3.1 is the right cut-off point.
src/vbox/vbox_tmpl.c | 31 ++-
1 files changed, 30 insertions(+), 1 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 2eb23fb..ba2252c 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -5586,9 +5586,12 @@ vboxDomainSnapshotGet(vboxGlobalData *data,
IMachine *machine,
const char *name)
{
- ISnapshot **snapshots = NULL;
ISnapshot *snapshot = NULL;
nsresult rc;
+
+#if VBOX_API_VERSION 3001
+
+ ISnapshot **snapshots = NULL;
int count = 0;
int i;
@@ -5615,6 +5618,30 @@ vboxDomainSnapshotGet(vboxGlobalData *data,
break;
}
+#else /* VBOX_API_VERSION = 3001 */
+ PRUnichar *nameUtf16 = NULL;
+
+ VBOX_UTF8_TO_UTF16(name, nameUtf16);
+ if (!nameUtf16) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+# if VBOX_API_VERSION 4000
+ rc = machine-vtbl-GetSnapshot(machine, nameUtf16, snapshot);
+# else /* VBOX_API_VERSION = 4000 */
+ rc = machine-vtbl-FindSnapshot(machine, nameUtf16, snapshot);
+# endif /* VBOX_API_VERSION = 4000 */
GetSnapshot with a name is wrong. According to the docs GetSnapshot
takes an UUID and FindSnapshot takes a name. GetSnapshot also takes
NULL as UUID and returns the first snapshot then. GetSnapshot and
FindSnapshot have been there since version 2.2 and probably earlier.
In version 4.0 FindSnapshot and GetSnapshot have been merged into
FindSnapshot that now takes a UUID or a name.
So as you're doing a name lookup here GetSnapshot is wrong and
FindSnapshot has to be used independent from the API version.
I only tested this with VBox 4.0 and it worked, but it'll fail for
VBox 4.0 I think, because of the use of GetSnapshot.
As FindSnapshot is available on all VBox versions this function should
probably use it unconditional and not fallback to
vboxDomainSnapshotGetAll for version 3.1. But as stated I didn't
test this yet.
+ VBOX_UTF16_FREE(nameUtf16);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _(could not get root snapshot for domain %s),
This error message it wrong, in this branch you never tried to get the root.
+ dom-name);
+ goto cleanup;
+ }
+
+#endif /* VBOX_API_VERSION = 3001 */
+
if (!snapshot) {
vboxError(VIR_ERR_OPERATION_INVALID,
While at it, this error code is wrong, it should be VIR_ERR_NO_DOMAIN_SNAPSHOT.
Therefore, I suggest this diff on top of this patch to fix the
mentioned problems.
ACK, with this diff applied.
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 901799b..666d59d 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -5627,23 +5627,16 @@ vboxDomainSnapshotGet(vboxGlobalData *data,
goto cleanup;
}
-# if VBOX_API_VERSION 4000
-rc = machine-vtbl-GetSnapshot(machine, nameUtf16, snapshot);
-# else /* VBOX_API_VERSION = 4000 */
rc = machine-vtbl-FindSnapshot(machine, nameUtf16, snapshot);
-# endif /* VBOX_API_VERSION = 4000 */
VBOX_UTF16_FREE(nameUtf16);
if (NS_FAILED(rc)) {
-vboxError(VIR_ERR_INTERNAL_ERROR,
- _(could not get root snapshot for domain %s),
- dom-name);
-goto cleanup;
+snapshot = NULL;
}
#endif /* VBOX_API_VERSION = 3001 */
if (!snapshot) {
-vboxError(VIR_ERR_OPERATION_INVALID,
+vboxError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
_(domain %s has no snapshots with name %s),
dom-name, name);
goto cleanup;
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] snapshot: implement getparent for vbox
2011/9/29 Eric Blake ebl...@redhat.com:
Again, built by copying from existing functions.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotGetParent): New function.
---
I can only compile-test this; I'm relying on someone with an actual
vbox setup to actually test it.
This patch needs some fixes, see below.
Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)
Is virDomainSnapshotNumChildren supposed to only return the number of
direct children, or all children (direct children, grandchildren, etc)
of a snapshot? In the later case they'll have to be counted
recursively, unfortunately.
src/vbox/vbox_tmpl.c | 71
++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index b483d0f..3d5f4ae 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6046,6 +6046,76 @@ cleanup:
}
static virDomainSnapshotPtr
+vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
+ unsigned int flags)
+{
+ virDomainPtr dom = snapshot-domain;
+ VBOX_OBJECT_CHECK(dom-conn, virDomainSnapshotPtr, NULL);
+ vboxIID iid = VBOX_IID_INITIALIZER;
+ IMachine *machine = NULL;
+ ISnapshot *snap = NULL;
+ ISnapshot *parent = NULL;
+ PRUnichar *nameUtf16 = NULL;
+ char *name = NULL;
+ nsresult rc;
+
+ virCheckFlags(0, NULL);
+
+ vboxIIDFromUUID(iid, dom-uuid);
+ rc = VBOX_OBJECT_GET_MACHINE(iid.value, machine);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_NO_DOMAIN, %s,
+ _(no domain with matching UUID));
+ goto cleanup;
+ }
+
+ if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot-name)))
+ goto cleanup;
At this point you already have the snapshot you want.
+ rc = machine-vtbl-GetCurrentSnapshot(machine, snap);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(could not get current snapshot));
+ goto cleanup;
+ }
This block here gives you the current snapshot, that's not what you want.
+ rc = snap-vtbl-GetParent(snap, parent);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _(could not get parent of snapshot %s),
+ snapshot-name);
+ goto cleanup;
+ }
+ if (!parent) {
+ vboxError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
+ _(snapshot '%s' does not have a parent),
+ snapshot-name);
+ goto cleanup;
+ }
+
+ rc = parent-vtbl-GetName(parent, nameUtf16);
+ if (NS_FAILED(rc) || !nameUtf16) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _(could not get name of parent of snapshot %s),
+ snapshot-name);
+ goto cleanup;
+ }
+ VBOX_UTF16_TO_UTF8(nameUtf16, name);
+ VBOX_UTF16_FREE(nameUtf16);
Because VBOX_UTF16_FREE doesn't set the pointer to NULL, calling it
twice on the same pointer isn't safe.
+
+ ret = virGetDomainSnapshot(dom, name);
+
+cleanup:
+ VBOX_UTF8_FREE(name);
+ VBOX_UTF16_FREE(nameUtf16);
And this second call segfaults in the success path.
+ VBOX_RELEASE(snap);
+ VBOX_RELEASE(parent);
+ VBOX_RELEASE(machine);
+ vboxIIDUnalloc(iid);
+ return ret;
+}
ACK with this diff applied:
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3d5f4ae..2eb23fb 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6072,13 +6072,6 @@
vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot-name)))
goto cleanup;
-rc = machine-vtbl-GetCurrentSnapshot(machine, snap);
-if (NS_FAILED(rc)) {
-vboxError(VIR_ERR_INTERNAL_ERROR, %s,
- _(could not get current snapshot));
-goto cleanup;
-}
-
rc = snap-vtbl-GetParent(snap, parent);
if (NS_FAILED(rc)) {
vboxError(VIR_ERR_INTERNAL_ERROR,
@@ -6101,7 +6094,10 @@
vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
goto cleanup;
}
VBOX_UTF16_TO_UTF8(nameUtf16, name);
-VBOX_UTF16_FREE(nameUtf16);
+if (!name) {
+virReportOOMError();
+goto cleanup;
+}
ret = virGetDomainSnapshot(dom, name);
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: implement getparent for esx
2011/9/29 Eric Blake ebl...@redhat.com: Pretty easy to paste together compared to existing functions. * src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function. --- I can only compile-test this; I'm relying on someone with an actual esx setup to actually test it. Also, I didn't see anything in existing code that would efficiently implement virDomainSnapshotNumChildren; there may an API that I'm not aware of, but someone else will have to implement that API (Matthias?) This will have to be done by esxVI_GetNumberOfSnapshotTrees that just counts the snapshots in the driver. This is one of things where vSphere API doesn't translate efficiently to libvirt API. src/esx/esx_driver.c | 41 + 1 files changed, 41 insertions(+), 0 deletions(-) Looks good, tested, works, ACK :) -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: implement getparent for esx
2011/9/30 Daniel Veillard veill...@redhat.com: On Thu, Sep 29, 2011 at 02:38:24PM -0600, Eric Blake wrote: Pretty easy to paste together compared to existing functions. * src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function. --- I can only compile-test this; I'm relying on someone with an actual esx setup to actually test it. Also, I didn't see anything in existing code that would efficiently implement virDomainSnapshotNumChildren; there may an API that I'm not aware of, but someone else will have to implement that API (Matthias?) The 2 patches looks just fine to me but agreed someone with the setup should check those and give the final ACK :-) Daniel I'll probably have some time this weekend to have a look at it. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Report an error for acceptable URI schemes with a transport
2011/9/27 Eric Blake ebl...@redhat.com:
On 09/27/2011 05:12 AM, Matthias Bolte wrote:
Before, URIs such as esx+ssh:// have been declined by the ESX driver
resulting in the remote driver trying to connect to an non-existing
libvirtd.
Now such URIs trigger and error in the ESX driver suggesting to try
again without the transport part in the scheme.
---
src/esx/esx_driver.c | 31 ++-
1 files changed, 26 insertions(+), 5 deletions(-)
Makes sense.
+ } else {
+ if (plus - conn-uri-scheme != 3 ||
+ (STRCASENEQLEN(conn-uri-scheme, vpx, 3)
+ STRCASENEQLEN(conn-uri-scheme, esx, 3)
+ STRCASENEQLEN(conn-uri-scheme, gsx, 3))) {
+ return VIR_DRV_OPEN_DECLINED;
+ }
+
+ ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
+ _(Transport in URI scheme is not supported, try again
+ without the transport part));
Maybe show the rejected transport in the error message:
ESX_ERROR(VIR_ERR_INVALID_ARG,
_(Transport '%s' in URI scheme is not supported, try again
without the transport part), plus + 1);
ACK, whether or not you make a change along those lines.
That's a good idea, it makes the error message more explicit. I folded
that in and pushed it.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hyperv: Report an error for acceptable URI schemes with a transport
2011/9/27 Eric Blake ebl...@redhat.com: On 09/27/2011 05:14 AM, Matthias Bolte wrote: Before, URIs such as hyperv+ssh:// have been declined by the Hyper-V driver resulting in the remote driver trying to connect to an non-existing libvirtd. Now such URIs trigger an error in the Hyper-V driver suggesting to try again without the transport part in the scheme. --- src/hyperv/hyperv_driver.c | 25 ++--- 1 files changed, 22 insertions(+), 3 deletions(-) Same review comments and ACK as for esx, regarding the error message quality. And same change applied. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Report an error for acceptable URI schemes with a transport
Before, URIs such as esx+ssh:// have been declined by the ESX driver
resulting in the remote driver trying to connect to an non-existing
libvirtd.
Now such URIs trigger and error in the ESX driver suggesting to try
again without the transport part in the scheme.
---
src/esx/esx_driver.c | 31 ++-
1 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f1102ea..c81eca8 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -938,20 +938,41 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
unsigned int flags)
{
virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
+char *plus;
esxPrivate *priv = NULL;
char *potentialVCenterIpAddress = NULL;
char vCenterIpAddress[NI_MAXHOST] = ;
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-/* Decline if the URI is NULL or the scheme is not one of {vpx|esx|gsx} */
-if (conn-uri == NULL || conn-uri-scheme == NULL ||
-(STRCASENEQ(conn-uri-scheme, vpx)
- STRCASENEQ(conn-uri-scheme, esx)
- STRCASENEQ(conn-uri-scheme, gsx))) {
+/* Decline if the URI is NULL or the scheme is NULL */
+if (conn-uri == NULL || conn-uri-scheme == NULL) {
return VIR_DRV_OPEN_DECLINED;
}
+/* Decline if the scheme is not one of {vpx|esx|gsx} */
+plus = strchr(conn-uri-scheme, '+');
+
+if (plus == NULL) {
+if (STRCASENEQ(conn-uri-scheme, vpx)
+STRCASENEQ(conn-uri-scheme, esx)
+STRCASENEQ(conn-uri-scheme, gsx)) {
+return VIR_DRV_OPEN_DECLINED;
+}
+} else {
+if (plus - conn-uri-scheme != 3 ||
+(STRCASENEQLEN(conn-uri-scheme, vpx, 3)
+ STRCASENEQLEN(conn-uri-scheme, esx, 3)
+ STRCASENEQLEN(conn-uri-scheme, gsx, 3))) {
+return VIR_DRV_OPEN_DECLINED;
+}
+
+ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
+ _(Transport in URI scheme is not supported, try again
+without the transport part));
+return VIR_DRV_OPEN_ERROR;
+}
+
/* Require server part */
if (conn-uri-server == NULL) {
ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] hyperv: Report an error for acceptable URI schemes with a transport
Before, URIs such as hyperv+ssh:// have been declined by the Hyper-V
driver resulting in the remote driver trying to connect to an
non-existing libvirtd.
Now such URIs trigger an error in the Hyper-V driver suggesting to
try again without the transport part in the scheme.
---
src/hyperv/hyperv_driver.c | 25 ++---
1 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index b022fee..c26d29f 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -69,6 +69,7 @@ static virDrvOpenStatus
hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
{
virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
+char *plus;
hypervPrivate *priv = NULL;
char *username = NULL;
char *password = NULL;
@@ -77,12 +78,30 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth,
unsigned int flags)
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-/* Decline if the URI is NULL or the scheme is not hyperv */
-if (conn-uri == NULL || conn-uri-scheme == NULL ||
-STRCASENEQ(conn-uri-scheme, hyperv)) {
+/* Decline if the URI is NULL or the scheme is NULL */
+if (conn-uri == NULL || conn-uri-scheme == NULL) {
return VIR_DRV_OPEN_DECLINED;
}
+/* Decline if the scheme is not hyperv */
+plus = strchr(conn-uri-scheme, '+');
+
+if (plus == NULL) {
+if (STRCASENEQ(conn-uri-scheme, hyperv)) {
+return VIR_DRV_OPEN_DECLINED;
+}
+} else {
+if (plus - conn-uri-scheme != 6 ||
+STRCASENEQLEN(conn-uri-scheme, hyperv, 6)) {
+return VIR_DRV_OPEN_DECLINED;
+}
+
+HYPERV_ERROR(VIR_ERR_INVALID_ARG, %s,
+ _(Transport in URI scheme is not supported, try again
+ without the transport part));
+return VIR_DRV_OPEN_ERROR;
+}
+
/* Require server part */
if (conn-uri-server == NULL) {
HYPERV_ERROR(VIR_ERR_INVALID_ARG, %s,
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] vmx: avoid memory leak
2011/9/18 a...@redhat.com:
* src/vmx/vmx.c: fix memory leak.
Signed-off-by: Alex Jia a...@redhat.com
---
src/vmx/vmx.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index dff3599..1ab15b7 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1253,6 +1253,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps,
const char *vmx)
/* Allocate domain def */
if (VIR_ALLOC(def) 0) {
virReportOOMError();
+ virConfFree(conf);
return NULL;
}
Your patch is correct.
But I'd suggest to replace the 'return NULL' with a 'goto cleanup'
here instead of adding a virConfFree, as this is more in line with the
surrounding code.
Either way ACK.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix managed object lookup with optional occurrence
2011/9/7 Eric Blake ebl...@redhat.com: On 09/06/2011 09:01 PM, Matthias Bolte wrote: Exit early if managed object is not found, instead of dereferencing a NULL pointer and triggering a segfault. --- src/esx/esx_vi.c | 8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Support folders in the path of vpx:// connection URIs
Allow the datacenter and compute resource parts of the path
to be prefixed with folders. Therefore, the way the path is
parsed has changed. Before, it was split in 2 or 3 items and
the items' meanings were determined by their positions. Now
the path can have 2 or more items and the the vCenter server
is asked whether a folder, datacenter of compute resource
with the specified name exists at the current hierarchy level.
Before the datacenter and compute resource lookup automatically
traversed folders during lookup. This is logic got removed
and folders have to be specified explicitly.
The proper datacenter path including folders is now used when
accessing a datastore over HTTPS. This makes virsh dumpxml
and define work for datacenters in folders.
https://bugzilla.redhat.com/show_bug.cgi?id=732676
---
docs/drvesx.html.in| 10 ++-
src/esx/esx_driver.c | 14 ++--
src/esx/esx_util.c | 19 +---
src/esx/esx_util.h |1 +
src/esx/esx_vi.c | 223 ++--
src/esx/esx_vi_generator.input |4 +
6 files changed, 215 insertions(+), 56 deletions(-)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index da9d2a1..aa8ecd4 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -56,7 +56,7 @@ esx://example-esx.com/?no_verify=1 (ESX over HTTPS, but
doesn't verify the s
URIs have this general form (code[...]/code marks an optional
part).
/p
pre
-type://[username@]hostname[:port]/[datacenter[/cluster]/server][?extraparameters]
+type://[username@]hostname[:port]/[[folder/...]datacenter/[folder/...][cluster/]server][?extraparameters]
/pre
p
The codetype:///code is either codeesx:///code or
@@ -80,6 +80,14 @@
type://[username@]hostname[:port]/[datacenter[/cluster]/server][?extraparameters
pre
vpx://example-vcenter.com/dc1/cluster1/example-esx.com
/pre
+p
+Datacenters and clusters can be organized in folders, those have to be
+specified as well. The driver can handle folders
+span class=sincesince 0.9.5/span.
+/p
+pre
+vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com
+/pre
h4a name=extraparamsExtra parameters/a/h4
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f1102ea..fd83a9c 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -802,8 +802,7 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr
auth,
char *url = NULL;
if (hostSystemIpAddress == NULL
-(priv-parsedUri-path_datacenter == NULL ||
- priv-parsedUri-path_computeResource == NULL)) {
+(priv-parsedUri-path == NULL || STREQ(priv-parsedUri-path, /))) {
ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
_(Path has to specify the datacenter and compute
resource));
return -1;
@@ -890,8 +889,8 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr
auth,
/*
- * URI format:
{vpx|esx|gsx}://[username@]hostname[:port]/[path][?query parameter
...]
- * path = datacenter/computeresource[/hostsystem]
+ * URI format:
{vpx|esx|gsx}://[username@]hostname[:port]/[path][?query parameter...]
+ * path =
[folder/...]datacenter/[folder/...]computeresource[/hostsystem]
*
* If no port is specified the default port is set dependent on the scheme and
* transport parameter:
@@ -905,7 +904,8 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr
auth,
* For a vpx:// connection path references a host managed by the vCenter.
* In case the host is part of a cluster then computeresource is the cluster
* name. Otherwise computeresource and hostsystem are equal and the later
- * can be omitted.
+ * can be omitted. As datacenters and computeresources can be organized in
+ * folders those have to be included in path.
*
* Optional query parameters:
* - transport={http|https}
@@ -2744,7 +2744,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int
flags)
domain-conn-uri-server, domain-conn-uri-port);
virBufferURIEncodeString(buffer, directoryAndFileName);
virBufferAddLit(buffer, ?dcPath=);
-virBufferURIEncodeString(buffer, priv-primary-datacenter-name);
+virBufferURIEncodeString(buffer, priv-parsedUri-path_datacenter);
virBufferAddLit(buffer, dsName=);
virBufferURIEncodeString(buffer, datastoreName);
@@ -3212,7 +3212,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml)
virBufferURIEncodeString(buffer, escapedName);
virBufferAddLit(buffer, .vmx?dcPath=);
-virBufferURIEncodeString(buffer, priv-primary-datacenter-name);
+virBufferURIEncodeString(buffer, priv-parsedUri-path_datacenter);
virBufferAddLit(buffer, dsName=);
virBufferURIEncodeString(buffer, datastoreName);
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index c14179d..5243a0e 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -51,7 +51,6 @@ esxUtil_ParseUri(esxUtil_ParsedUri
Re: [libvirt] [PATCH] mingw: Don't use interface as an identifier
2011/9/6 Eric Blake ebl...@redhat.com: On 09/06/2011 07:44 AM, Matthias Bolte wrote: Because it's a define use in MSCOM and its usage as identifier results in a compile error. Lame of the headers to invade the namespace like that, but easy enough to work around. --- tools/virsh.c | 24 ++-- 1 files changed, 10 insertions(+), 14 deletions(-) ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix managed object lookup with optional occurrence
Exit early if managed object is not found, instead of dereferencing
a NULL pointer and triggering a segfault.
---
src/esx/esx_vi.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 5c8d79e..f4033eb 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -3964,7 +3964,7 @@
esxVI_ProductVersionToDefaultVirtualHWVersion(esxVI_ProductVersion productVersio
#define ESX_VI__TEMPLATE__LOOKUP(_type, _complete_properties, \
- _cast_from_anytype) \
+ _cast_from_anytype) \
int \
esxVI_Lookup##_type(esxVI_Context *ctx, const char* name /* optional */, \
esxVI_ManagedObjectReference *root, \
@@ -3999,6 +3999,12 @@
esxVI_ProductVersionToDefaultVirtualHWVersion(esxVI_ProductVersion productVersio
goto cleanup; \
} \
\
+if (objectContent == NULL) { \
+/* not found, exit early */ \
+result = 0; \
+goto cleanup; \
+} \
+ \
if (esxVI_##_type##_Alloc(ptrptr) 0) { \
goto cleanup; \
} \
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/5] hyperv: Add OpenWSMAN based client for the Hyper-V WMI API
2011/8/20 Eric Blake ebl...@redhat.com:
On 08/03/2011 09:00 AM, Matthias Bolte wrote:
Add a generator script to generate the structs and serialization
information for OpenWSMAN.
openwsman.h collects workarounds for problems in OpenWSMAN= 2.2.6.
There are also disabled sections that would use ws_serializer_free_mem
but can't because it's broken in OpenWSMAN= 2.2.6. Patches to fix
this have been posted upstream.
---
v2:
- no changes
+
+
+int
+hyperyVerifyResponse(WsManClient *client, WsXmlDocH response,
+ const char *detail)
+{
+ int lastError = wsmc_get_last_error(client);
+ int responseCode = wsmc_get_response_code(client);
+ WsManFault *fault;
+
+ if (lastError != WS_LASTERR_OK) {
+ HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _(Transport error during %s: %s (%d)),
+ detail,
wsman_transport_get_last_error_string(lastError),
+ lastError);
+ return -1;
+ }
+
+ if (responseCode != 200 responseCode != 400 responseCode !=
500) {
Magic numbers. Should these have names?
As there are used only once here, I'll add a comment, instead of an
enum for now.
+int
+hypervMsvmComputerSystemEnabledStateToDomainState
+ (Msvm_ComputerSystem *computerSystem)
+{
+ switch (computerSystem-data-EnabledState) {
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_UNKNOWN:
+ return VIR_DOMAIN_NOSTATE;
+
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED:
+ return VIR_DOMAIN_RUNNING;
+
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_DISABLED:
+ return VIR_DOMAIN_SHUTOFF;
+
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_PAUSED:
+ return VIR_DOMAIN_PAUSED;
+
+ case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED:
+ return VIR_DOMAIN_SHUTOFF;
Is suspended really more like shutoff or paused? What's the distinction
being used by hyperv for these states?
Suspended maps to libvirt's managed save.
+++ b/src/hyperv/hyperv_wmi_generator.input
@@ -0,0 +1,294 @@
+#
+# Definitions of WMI classes used as input for the
hyperv_wmi_generator.py
+# script.
+#
+# This format is line-based, so end-of-line is important.
+#
+#
+# Class definition:
+#
+# classname
+#type name
+# ...
+# end
+#
+# Allowed values fortype are: boolean, string, datetime, int8, int16,
+# int32, int64, uint8, uint16, uint32 and uint64
+#
+# The propertyname can be followed by [] to define a dynamic array.
+#
+
+
+class Msvm_ComputerSystem
+ string Caption
I'm trusting that these definitions match a documented layout somewhere. Is
there a reference URL worth embedding as a comment here, in case we need to
refer back to the original struct definition that this copies from?
They are all documented in MSDN, I'll add a link to it.
+++ b/src/hyperv/hyperv_wmi_generator.py
@@ -0,0 +1,309 @@
+#!/usr/bin/env python
My python's weak, but I did glance through all of the generated files, and
they look pretty sane, so I'm acking on the basis of the output.
+def main():
+ if srcdir in os.environ:
+ input_filename = os.path.join(os.environ[srcdir],
hyperv/hyperv_wmi_generator.input)
+ output_dirname = os.path.join(os.environ[srcdir], hyperv)
+ else:
+ input_filename = os.path.join(os.getcwd(),
hyperv_wmi_generator.input)
+ output_dirname = os.getcwd()
I'm hoping this plays well with VPATH builds (so far, I've only tested
in-tree builds).
This generator is based on the ESX one, therefore it should work in
VPATH builds, otherwise you should have seen problems with the ESX
generator in VPATH builds in the past.
Also the else case with getcwd will only be executed when the
generator is run manually, as the Makefile always passes srcdir to it.
+#ifndef __OPENWSMAN_H__
+# define __OPENWSMAN_H__
+
+/* Workaround openwsman= 2.2.6 unconditionally defining optarg. Just
pretend
+ * that u/os.h was already included. Need to explicitly include time.h
because
+ * wsman-xml-serializer.h needs it and u/os.h would have included it. */
+# includetime.h
+# define _LIBU_OS_H_
+# includewsman-api.h
Oh the gross things we have to do. The comment is worth its weight in gold.
I'm currently in the process of getting some patches into openwsman
upstream to fix those problems.
I pointed out a couple of nits, but nothing jumped out at me as a
showstopper. ACK, and I'm happy if you push without posting a v3.
Here's the interdiff I'm going to apply, before I push the whole series.
--
Matthias Bolte
http://photron.blogspot.com
diff --git a/.gitignore b/.gitignore
index 39ecd6d..1e8d5ab 100644
--- a/.gitignore
+++ b/.gitignore
@@ -54,6 +54,7 @@
/proxy/
/python/generator.py.stamp
/sc_*
+/src/hyperv/*.generated.*
/src/libvirt_iohelper
/src/locking/qemu-sanlock.conf
/src/remote/*_client_bodies.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 796e6b8..c8605fc 100644
--- a/src/Makefile.am
+++ b/src
Re: [libvirt] [PATCHv2 4/5] hyperv: Add basic driver for Microsoft Hyper-V
2011/8/20 Eric Blake ebl...@redhat.com:
On 08/03/2011 09:00 AM, Matthias Bolte wrote:
Domain listing, basic information retrieval and domain life cycle
management is implemented. But currently the domian XML output
s/domian/domain/
lacks the complete devices section.
The driver uses OpenWSMAN to directly communicate with an Hyper-V
s/an Hyper/a Hyper/
Since the H in Hyper is aspirated, we use 'a' instead of 'an'. Blame
English for being stupid.
server over its WS-Management interface exposed via Microsoft WinRM.
The driver is based on the work of Michael Sievers. This started in
the same master program project group at the University of Paderborn
as the ESX driver.
See Michael's blog for details: http://hyperv4libvirt.wordpress.com/
---
+ /* Strip the string to fit more relevant information in 32 chars */
+ tmp = processorList-data-Name;
+
+ while (*tmp != '\0') {
+ if (STRPREFIX(tmp, )) {
+ memmove(tmp, tmp + 1, strlen(tmp + 1) + 1);
+ continue;
+ } else if (STRPREFIX(tmp, (R)) || STRPREFIX(tmp, (C))) {
+ memmove(tmp, tmp + 3, strlen(tmp + 3) + 1);
+ continue;
+ } else if (STRPREFIX(tmp, (TM))) {
Cute. Hopefully no one complains about stripping copyright and trademark
notations in our compressed strings. I think you're okay, but IANAL.
This is the same logic as in the ESX driver.
Conditional ACK - I pointed out a couple more flag-variant functions that
are trivially implemented. I'm okay if you post a delta patch for review,
then squash it in before pushing, since that would be shorter than a
full-blown v3.
Here's an interdiff for v3.
--
Matthias Bolte
http://photron.blogspot.com
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 5301ec5..b022fee 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -164,7 +164,7 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
/* Check if the connection can be established and if the server has the
* Hyper-V role installed. If the call to hypervGetMsvmComputerSystemList
- * succeeds than the connection has be established. If the returned list
+ * succeeds than the connection has been established. If the returned list
* is empty than the server isn't a Hyper-V server. */
virBufferAddLit(query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
virBufferAddLit(query, where );
@@ -594,13 +594,15 @@ hypervDomainResume(virDomainPtr domain)
static int
-hypervDomainDestroy(virDomainPtr domain)
+hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags)
{
int result = -1;
hypervPrivate *priv = domain-conn-privateData;
Msvm_ComputerSystem *computerSystem = NULL;
bool in_transition = false;
+virCheckFlags(0, -1);
+
if (hypervMsvmComputerSystemFromDomain(domain, computerSystem) 0) {
goto cleanup;
}
@@ -623,6 +625,14 @@ hypervDomainDestroy(virDomainPtr domain)
+static int
+hypervDomainDestroy(virDomainPtr domain)
+{
+return hypervDomainDestroyFlags(domain, 0);
+}
+
+
+
static char *
hypervDomainGetOSType(virDomainPtr domain ATTRIBUTE_UNUSED)
{
@@ -787,6 +797,8 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
Msvm_ProcessorSettingData *processorSettingData = NULL;
Msvm_MemorySettingData *memorySettingData = NULL;
+/* Flags checked by virDomainDefFormat */
+
if (VIR_ALLOC(def) 0) {
virReportOOMError();
goto cleanup;
@@ -1022,12 +1034,14 @@ hypervNumberOfDefinedDomains(virConnectPtr conn)
static int
-hypervDomainCreate(virDomainPtr domain)
+hypervDomainCreateWithFlags(virDomainPtr domain, unsigned int flags)
{
int result = -1;
hypervPrivate *priv = domain-conn-privateData;
Msvm_ComputerSystem *computerSystem = NULL;
+virCheckFlags(0, -1);
+
if (hypervMsvmComputerSystemFromDomain(domain, computerSystem) 0) {
goto cleanup;
}
@@ -1050,6 +1064,14 @@ hypervDomainCreate(virDomainPtr domain)
static int
+hypervDomainCreate(virDomainPtr domain)
+{
+return hypervDomainCreateWithFlags(domain, 0);
+}
+
+
+
+static int
hypervIsEncrypted(virConnectPtr conn)
{
hypervPrivate *priv = conn-privateData;
@@ -1218,6 +1240,7 @@ static virDriver hypervDriver = {
.domainSuspend = hypervDomainSuspend, /* 0.9.5 */
.domainResume = hypervDomainResume, /* 0.9.5 */
.domainDestroy = hypervDomainDestroy, /* 0.9.5 */
+.domainDestroyFlags = hypervDomainDestroyFlags, /* 0.9.5 */
.domainGetOSType = hypervDomainGetOSType, /* 0.9.5 */
.domainGetInfo = hypervDomainGetInfo, /* 0.9.5 */
.domainGetState = hypervDomainGetState, /* 0.9.5 */
@@ -1225,6 +1248,7 @@ static virDriver hypervDriver = {
.listDefinedDomains = hypervListDefinedDomains, /* 0.9.5 */
.numOfDefinedDomains = hypervNumberOfDefinedDomains, /* 0.9.5 */
.domainCreate = hypervDomainCreate, /* 0.9.5
Re: [libvirt] [PATCHv2 5/5] hyperv: Add basic documentation
2011/8/20 Eric Blake ebl...@redhat.com: On 08/03/2011 09:00 AM, Matthias Bolte wrote: --- v2: - move microsoft.com link to drvhyperv.html.in docs/drivers.html.in | 1 + docs/drvhyperv.html.in | 112 docs/index.html.in | 3 + docs/sitemap.html.in | 4 ++ src/README | 3 +- Thanks! Too often, the docs get overlooked. +h2a name=uriConnections to the Microsoft Hyper-V driver/a/h2 +p + Some example remote connection URIs for the driver are: +/p +pre +hyperv://example-hyperv.com (over HTTPS) +hyperv://example-hyperv.com/?transport=http (over HTTP) Wouldn't it be more typical to represent that as: hyperv+http://example-hyperv.com/ that is, qemu+ssh://... uses ssh as the transport, rather than qemu://host/?transport=ssh Well, this was modeled after the way the ESX driver does it. In retrospect I'm not sure why I did it this way in the ESX driver. The HTTP transport in the ESX driver is mainly for debugging by tcpdump'ing the communication and the HTTP transport in the Hyper-V driver is there because this was simpler than configuring my Hyper-V server for HTTPS :) +/pre +p +strongNote/strong: In contrast to other drivers, the Hyper-V driver + is a client-side-only driver. It connects to the Hyper-V server using + WS-Management over HTTP(S). Therefore, the +a href=remote.htmlremote transport mechanism/a provided by the + remote driver and libvirtd will not work, and you cannot use URIs like +codehyperv+ssh://example.com/code. Then again, I guess I see why not. +p + To allowcodeBasic/code authentication with HTTP transport WinRM + needs to allow unencrypted communication. This can be enabled via the + WinRM commandline tool. Although this is not the recommended + communication mode. s/Although/However,/ ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/5] Add basic driver for Microsoft Hyper-V
2011/8/20 Eric Blake ebl...@redhat.com: On 08/03/2011 09:00 AM, Matthias Bolte wrote: Version 2 of the Hyper-V driver. See the individual patches for the changes from version 1. Complete version 1 can be found here https://www.redhat.com/archives/libvir-list/2011-July/msg00766.html This series adds the basic driver and supports listing and retrieving information about existing domains and their lifecycle management. Let's get this in for 0.9.5. Looks like a good start. Okay, after sorting out all comments on the patches I pushed the whole series now, thanks. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/5] hyperv: Add OpenWSMAN based client for the Hyper-V WMI API
2011/8/26 Eric Blake ebl...@redhat.com:
On 08/26/2011 06:10 AM, Matthias Bolte wrote:
I pointed out a couple of nits, but nothing jumped out at me as a
showstopper. ACK, and I'm happy if you push without posting a v3.
Here's the interdiff I'm going to apply, before I push the whole series.
Looks good. ACK to the interdiff.
@@ -61,6 +63,8 @@ hyperyVerifyResponse(WsManClient *client, WsXmlDocH
response,
return -1;
}
+ /* Check the HTTP response code and report an error if it's not 200
(OK),
+ * 400 (Bad Request) or 500 (Internal Server Error) */
if (responseCode != 200 responseCode != 400 responseCode !=
500) {
I'm wondering if we might ever want to check for other codes, like 401, but
that doesn't hold up this patch as-is.
The basic logic and the error codes in this function comes from a
helper function in openwsman, that i recreated here with some
additional logic.
401 for example is handled else where in openwsman, as far as I can
tell from the codebase.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Use $(PYTHON) instead of the shebang to run the generator
2011/8/23 Eric Blake ebl...@redhat.com: On 08/23/2011 03:42 PM, Matthias Bolte wrote: --- src/Makefile.am | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8fe7120..5ba189c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -812,8 +812,9 @@ endif BUILT_SOURCES += $(ESX_DRIVER_GENERATED) -$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input $(srcdir)/esx/esx_vi_generator.py - $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py +$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input \ + $(srcdir)/esx/esx_vi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(PYTHON) $(srcdir)/esx/esx_vi_generator.py ACK. [Hmm - this looks like backports of my hyperv review comments ;] Exactly, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Refactor a repeated string in the generator
--- src/esx/esx_vi_generator.py | 24 +--- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 239ec73..8a128df 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1817,17 +1817,19 @@ for obj in managed_objects_by_name.values(): -types_typedef.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -types_typeenum.write(/* Generated by esx_vi_generator.py */\n\n) -types_typetostring.write(/* Generated by esx_vi_generator.py */\n\n) -types_typefromstring.write(/* Generated by esx_vi_generator.py */\n\n) -types_header.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -types_source.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -methods_header.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -methods_source.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -methods_macro.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -helpers_header.write(/* Generated by esx_vi_generator.py */\n\n\n\n) -helpers_source.write(/* Generated by esx_vi_generator.py */\n\n\n\n) +notice = /* Generated by esx_vi_generator.py */\n\n\n\n + +types_typedef.write(notice) +types_typeenum.write(notice) +types_typetostring.write(notice) +types_typefromstring.write(notice) +types_header.write(notice) +types_source.write(notice) +methods_header.write(notice) +methods_source.write(notice) +methods_macro.write(notice) +helpers_header.write(notice) +helpers_source.write(notice) -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Refactor a repeated string in the generator
2011/8/23 Eric Blake ebl...@redhat.com: On 08/23/2011 03:18 PM, Matthias Bolte wrote: --- src/esx/esx_vi_generator.py | 24 +--- 1 files changed, 13 insertions(+), 11 deletions(-) ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Use $(PYTHON) instead of the shebang to run the generator
--- src/Makefile.am |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8fe7120..5ba189c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -812,8 +812,9 @@ endif BUILT_SOURCES += $(ESX_DRIVER_GENERATED) -$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input $(srcdir)/esx/esx_vi_generator.py - $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py +$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input \ + $(srcdir)/esx/esx_vi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(PYTHON) $(srcdir)/esx/esx_vi_generator.py if WITH_ESX if WITH_DRIVER_MODULES -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Bug - libvirt persists on having dnsmasq
2011/8/16 Zdenek Styblik sty...@turnovfree.net: On 08/16/11 18:07, Zdenek Styblik wrote: [...] Well, libvirt.spec has had Require: dnsmasq for at least a couple years, so any install of a libvirt rpm should fail when dnsmasq isn't present. I don't know for certain how long the BuildRequires: dnsmasq has been there, but certainly for at least a year (the last time the line was modified), so any rpm builds should also fail. But of course if you're running ./autogen.sh and then make, that doesn't involve the specfile. dnsmasq really is an integral part of the network driver; I don't know that it makes any sense to fix things so it can be built without dnsmasq. It's probably a good idea to make the failure complete though. We already have an option to disable the virtual network driver. I don't think we want to have a separate option for dnsmasq, since that is an integral part of hte network driver IMHO Daniel I'm sorry, but what? I did not understand single word, and I mean it - no irony, no pun. In other words and what I took from the reply, you mean to tell me from libvirt-0.9.4 and on one is forced to have dnsmasq around resp. to use libvirt? Thanks, Z. So, let me just repeat whole thing again. Up until libvirt version 0.9.3, libvirt was able to auto-magically detect presence of dnsmasq. And in case dnsmasq was not found during ./configure; libvirt wouldn't try to use it. Actually I think that's not true. I think the code just accidentally ignored it when dnsmasq could not be started. This commit (part of libvirt 0.9.4) http://libvirt.org/git/?p=libvirt.git;a=commit;h=85a954ce3ae6210a779a5f3b29559ec5f862 fixed this. I didn't test it but this seems to be the cause for the problem you're describing. However, since libvirt version 0.9.4 resp. in libvirt version 0.9.4 this auto-magical detection does not work and libvirt persists on using dnsmasq which is not present. True, because a bug was fixed. It may look to you as there was a feature that's broken now, but that's not the case, I think. The code is expecting dnsmasq to be available when a virtual network is used. A simple solution for you might be to revert commit 85a954ce3ae6210a779a5f3b29559ec5f862 and go back to exploiting the bug, that made it work for you as you expect it. I don't know what to make from your replies as it seems like you don't want to get blame for this one nor there seems to be will not to make libvirt dnsmasq independent. I don't think that this is the case here. I think Dan and Laine weren't aware that there was a bug fixed in libvirt 0.9.4 that gave you the impression that dnsmasq was optional. I blame nobody. I want to know, I want to fix it one way or another, and move on. One could argue that the virtual network code could be more fine grained and only start dnsmasq when it's really necessary, as this is what you expect from it. Currently it tries to start dnsmasq when the network contains an ip element. See http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/network/bridge_driver.c#l1806 You might suggest that it only should start/require dnsmasq when there is a dhcp element or something DNS related that really makes dnsmasq necessary. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virNetClientPtr leak in remote driver
2011/8/2 Daniel P. Berrange berra...@redhat.com:
On Mon, Aug 01, 2011 at 06:35:21PM +0200, Matthias Bolte wrote:
2011/8/1 Eric Blake ebl...@redhat.com:
On 07/28/2011 12:07 PM, Matthias Bolte wrote:
2011/7/27 Matthias Bolte matthias.bo...@googlemail.com:
doRemoteClose doesn't free the virNetClientPtr and this creates a
260kb leak per remote connection. This happens because
virNetClientFree doesn't remove the last ref, because virNetClientNew
creates the virNetClientPtr with a refcount of 2.
The memory leak I saw was due to virsh calling
virEventRegisterDefaultImpl, but not calling virEventRunDefaultImpl.
Because the event loop is initialized, the call to
virNetSocketAddIOCallback in virNetClientNew succeeds. But virsh not
driving the event loop results in not removing callbacks that were
marked for deletion. Finally this leaks the virNetClientPtr using in
the remote driver. I used a virsh -c qemu:///system to test.
I was able fix this by calling virEventRunDefaultImpl after
virConnectClose in virsh. But I don't think that this is the correct
general solution.
Where do we stand with 0.9.4? Is this something where we need the
general fix before the release, or is just the virsh hack to call
virEventRunDefaultImpl good enough for the release, or is this problem
not severe enough and we can release as-is?
virsh is a bit special here as it registers/initializes the default
even loop but doesn't drive it properly. It only drives it for the
console command. That's the problem in virsh. This can be avoided by
calling virEventRunDefaultImpl after virConnectClose iff it's a remote
connection, in other cases virEventRunDefaultImpl might block.
Therefore, we shouldn't be calling it in general after a
virConnectClose in virsh.
If we assume that an application that registers an event loop will
drive it properly then this problem is not critical, as it doesn't
exist. Just virsh is a special case that leaks 260kb per closed remote
connection. When we assume that a typical virsh user uses a single
connection per virsh invocation then we can view this as a static
leak.
Yeah, this is a case I never thought of. The easy fix is to just
make virsh spawn a new thread to run the event loop in the background.
The problem here will probably be the console command with this lines
while (!con-quit) {
if (virEventRunDefaultImpl() 0)
break;
}
in console.c. Having two threads calling virEventRunDefaultImpl
probably isn't a good idea.
The hard fix is to make virsh I/O entirely event driven, so that
we don't just sit in a blocking read of stdin waiting for input,
but instead use the event loop to process stdin.
Daniel
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 3/5] hyperv: Add OpenWSMAN based client for the Hyper-V WMI API
, ...) \
virReportErrorHelper(VIR_FROM_HYPERV, code, __FILE__, __FUNCTION__, \
__LINE__, __VA_ARGS__)
+typedef struct _hypervPrivate hypervPrivate;
+
+struct _hypervPrivate {
+WsManClient *client;
+};
+
#endif /* __HYPERV_PRIVATE_H__ */
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
new file mode 100644
index 000..dcfc3f9
--- /dev/null
+++ b/src/hyperv/hyperv_wmi.c
@@ -0,0 +1,684 @@
+
+/*
+ * hyperv_wmi.h: general WMI over WSMAN related functions and structures for
+ * managing Microsoft Hyper-V hosts
+ *
+ * Copyright (C) 2011 Matthias Bolte matthias.bo...@googlemail.com
+ * Copyright (C) 2009 Michael Sievers msiever...@googlemail.com
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include config.h
+
+#include internal.h
+#include virterror_internal.h
+#include datatypes.h
+#include logging.h
+#include memory.h
+#include util.h
+#include uuid.h
+#include buf.h
+#include hyperv_private.h
+#include hyperv_wmi.h
+
+#define ROOT_CIMV2 \
+http://schemas.microsoft.com/wbem/wsman/1/wmi/root/cimv2/*;
+
+#define ROOT_VIRTUALIZATION \
+http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/*;
+
+#define VIR_FROM_THIS VIR_FROM_HYPERV
+
+
+
+int
+hyperyVerifyResponse(WsManClient *client, WsXmlDocH response,
+ const char *detail)
+{
+int lastError = wsmc_get_last_error(client);
+int responseCode = wsmc_get_response_code(client);
+WsManFault *fault;
+
+if (lastError != WS_LASTERR_OK) {
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _(Transport error during %s: %s (%d)),
+ detail, wsman_transport_get_last_error_string(lastError),
+ lastError);
+return -1;
+}
+
+if (responseCode != 200 responseCode != 400 responseCode != 500) {
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _(Unexpected HTTP response during %s: %d),
+ detail, responseCode);
+return -1;
+}
+
+if (response == NULL) {
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _(Empty response during %s), detail);
+return -1;
+}
+
+if (wsmc_check_for_fault(response)) {
+fault = wsmc_fault_new();
+
+if (fault == NULL) {
+virReportOOMError();
+return -1;
+}
+
+wsmc_get_fault_data(response, fault);
+
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR,
+ _(SOAP fault during %s: code '%s', subcode '%s',
+ reason '%s', detail '%s'),
+ detail, NULLSTR(fault-code), NULLSTR(fault-subcode),
+ NULLSTR(fault-reason), NULLSTR(fault-fault_detail));
+
+wsmc_fault_destroy(fault);
+return -1;
+}
+
+return 0;
+}
+
+
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Object
+ */
+
+int
+hypervEnumAndPull(hypervPrivate *priv, virBufferPtr query, const char *root,
+ XmlSerializerInfo *serializerInfo, const char *resourceUri,
+ const char *className, hypervObject **list)
+{
+int result = -1;
+WsSerializerContextH serializerContext;
+client_opt_t *options = NULL;
+char *query_string = NULL;
+filter_t *filter = NULL;
+WsXmlDocH response = NULL;
+char *enumContext = NULL;
+hypervObject *head = NULL;
+hypervObject *tail = NULL;
+WsXmlNodeH node = NULL;
+XML_TYPE_PTR data = NULL;
+hypervObject *object;
+
+if (list == NULL || *list != NULL) {
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR, %s, _(Invalid argument));
+return -1;
+}
+
+if (virBufferError(query)) {
+virReportOOMError();
+return -1;
+}
+
+serializerContext = wsmc_get_serialization_context(priv-client);
+
+options = wsmc_options_init();
+
+if (options == NULL) {
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Could not initialize options));
+goto cleanup;
+}
+
+query_string = virBufferContentAndReset(query);
+filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string);
+
+if (filter == NULL) {
+HYPERV_ERROR
[libvirt] [PATCHv2 5/5] hyperv: Add basic documentation
---
v2:
- move microsoft.com link to drvhyperv.html.in
docs/drivers.html.in |1 +
docs/drvhyperv.html.in | 112
docs/index.html.in |3 +
docs/sitemap.html.in |4 ++
src/README |3 +-
5 files changed, 122 insertions(+), 1 deletions(-)
create mode 100644 docs/drvhyperv.html.in
diff --git a/docs/drivers.html.in b/docs/drivers.html.in
index 0428870..75038fc 100644
--- a/docs/drivers.html.in
+++ b/docs/drivers.html.in
@@ -28,6 +28,7 @@
listronga href=drvesx.htmlVMware ESX/a/strong/li
listronga href=drvvmware.htmlVMware
Workstation/Player/a/strong/li
listronga href=drvxen.htmlXen/a/strong/li
+ listronga href=drvhyperv.htmlMicrosoft Hyper-V/a/strong/li
/ul
h2a name=stroageStorage drivers/a/h2
diff --git a/docs/drvhyperv.html.in b/docs/drvhyperv.html.in
new file mode 100644
index 000..f710090
--- /dev/null
+++ b/docs/drvhyperv.html.in
@@ -0,0 +1,112 @@
+htmlbody
+h1Microsoft Hyper-V hypervisor driver/h1
+ul id=toc/ul
+p
+The libvirt Microsoft Hyper-V driver can manage Hyper-V 2008 R2.
+/p
+
+
+h2a name=projectProject Links/a/h2
+ul
+ li
+The a href=http://www.microsoft.com/hyper-v-server/;Microsoft
Hyper-V/a
+hypervisor
+ /li
+/ul
+
+
+h2a name=uriConnections to the Microsoft Hyper-V driver/a/h2
+p
+Some example remote connection URIs for the driver are:
+/p
+pre
+hyperv://example-hyperv.com (over HTTPS)
+hyperv://example-hyperv.com/?transport=http (over HTTP)
+/pre
+p
+strongNote/strong: In contrast to other drivers, the Hyper-V driver
+is a client-side-only driver. It connects to the Hyper-V server using
+WS-Management over HTTP(S). Therefore, the
+a href=remote.htmlremote transport mechanism/a provided by the
+remote driver and libvirtd will not work, and you cannot use URIs like
+codehyperv+ssh://example.com/code.
+/p
+
+
+h3a name=uriformatURI Format/a/h3
+p
+URIs have this general form (code[...]/code marks an optional
part).
+/p
+pre
+hyperv://[username@]hostname[:port]/[?extraparameters]
+/pre
+p
+The default HTTPS ports is 5986. If the port parameter is given, it
+overrides the default port.
+/p
+
+
+h4a name=extraparamsExtra parameters/a/h4
+p
+Extra parameters can be added to a URI as part of the query string
+(the part following code?/code). A single parameter is formed by a
+codename=value/code pair. Multiple parameters are separated by
+codeamp;/code.
+/p
+pre
+?transport=http
+/pre
+p
+The driver understands the extra parameters shown below.
+/p
+table class=top_table
+tr
+thName/th
+thValues/th
+thMeaning/th
+/tr
+tr
+td
+codetransport/code
+/td
+td
+codehttp/code or codehttps/code
+/td
+td
+Overrides the default HTTPS transport. The default HTTP port
+is 5985.
+/td
+/tr
+/table
+
+
+h3a name=authAuthentication/a/h3
+p
+In order to perform any useful operation the driver needs to log into
+the Hyper-V server. Therefore, only codevirConnectOpenAuth/code can
+be used to connect to an Hyper-V server, codevirConnectOpen/code
and
+codevirConnectOpenReadOnly/code don't work.
+To log into an Hyper-V server the driver will request credentials using
+the callback passed to the codevirConnectOpenAuth/code function.
+The driver passes the hostname as challenge parameter to the callback.
+/p
+p
+strongNote/strong: Currently only codeBasic/code authentication
+is supported by libvirt. This method is disabled by default on the
+Hyper-V server and can be enabled via the WinRM commandline tool.
+/p
+pre
+winrm set winrm/config/service/auth @{Basic=true}
+/pre
+p
+To allow codeBasic/code authentication with HTTP transport WinRM
+needs to allow unencrypted communication. This can be enabled via the
+WinRM commandline tool. Although this is not the recommended
+communication mode.
+/p
+pre
+winrm set winrm/config/service @{AllowUnencrypted=true}
+/pre
+
+
+/body/html
diff --git a/docs/index.html.in b/docs/index.html.in
index 536e354..c84eb1f 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -60,6 +60,9 @@
The a href=http://libvirt.org/drvvmware.html;VMware Workstation and
Player/a hypervisors
/li
li
+The a href=http://libvirt.org/drvhyperv.html;Microsoft Hyper-V/a
hypervisor
+ /li
+ li
Virtual networks using bridging, NAT, VEPA and VN-LINK.
/li
li
diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in
[libvirt] [PATCHv2 4/5] hyperv: Add basic driver for Microsoft Hyper-V
);
+
+return result;
+}
+
+
+
static virDriver hypervDriver = {
.no = VIR_DRV_HYPERV,
.name = Hyper-V,
.open = hypervOpen, /* 0.9.5 */
.close = hypervClose, /* 0.9.5 */
+.type = hypervGetType, /* 0.9.5 */
+.getHostname = hypervGetHostname, /* 0.9.5 */
+.nodeGetInfo = hypervNodeGetInfo, /* 0.9.5 */
+.listDomains = hypervListDomains, /* 0.9.5 */
+.numOfDomains = hypervNumberOfDomains, /* 0.9.5 */
+.domainLookupByID = hypervDomainLookupByID, /* 0.9.5 */
+.domainLookupByUUID = hypervDomainLookupByUUID, /* 0.9.5 */
+.domainLookupByName = hypervDomainLookupByName, /* 0.9.5 */
+.domainSuspend = hypervDomainSuspend, /* 0.9.5 */
+.domainResume = hypervDomainResume, /* 0.9.5 */
+.domainDestroy = hypervDomainDestroy, /* 0.9.5 */
+.domainGetOSType = hypervDomainGetOSType, /* 0.9.5 */
+.domainGetInfo = hypervDomainGetInfo, /* 0.9.5 */
+.domainGetState = hypervDomainGetState, /* 0.9.5 */
+.domainGetXMLDesc = hypervDomainGetXMLDesc, /* 0.9.5 */
+.listDefinedDomains = hypervListDefinedDomains, /* 0.9.5 */
+.numOfDefinedDomains = hypervNumberOfDefinedDomains, /* 0.9.5 */
+.domainCreate = hypervDomainCreate, /* 0.9.5 */
+.isEncrypted = hypervIsEncrypted, /* 0.9.5 */
+.isSecure = hypervIsSecure, /* 0.9.5 */
+.domainIsActive = hypervDomainIsActive, /* 0.9.5 */
+.domainIsPersistent = hypervDomainIsPersistent, /* 0.9.5 */
+.domainIsUpdated = hypervDomainIsUpdated, /* 0.9.5 */
+.domainManagedSave = hypervDomainManagedSave, /* 0.9.5 */
+.domainHasManagedSaveImage = hypervDomainHasManagedSaveImage, /* 0.9.5 */
+.domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */
};
+static void
+hypervDebugHandler(const char *message, debug_level_e level,
+ void *user_data ATTRIBUTE_UNUSED)
+{
+switch (level) {
+ case DEBUG_LEVEL_ERROR:
+ case DEBUG_LEVEL_CRITICAL:
+VIR_ERROR(_(openwsman error: %s), message);
+break;
+
+ case DEBUG_LEVEL_WARNING:
+VIR_WARN(openwsman warning: %s, message);
+break;
+
+ default:
+/* Ignore the rest */
+break;
+}
+}
+
+
+
int
hypervRegister(void)
{
@@ -104,5 +1272,8 @@ hypervRegister(void)
return -1;
}
+/* Forward openwsman errors and warnings to libvirt's logging */
+debug_add_handler(hypervDebugHandler, DEBUG_LEVEL_WARNING, NULL);
+
return 0;
}
diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h
index 0d5370e..ebddf5d 100644
--- a/src/hyperv/hyperv_private.h
+++ b/src/hyperv/hyperv_private.h
@@ -26,6 +26,7 @@
# include internal.h
# include virterror_internal.h
+# include hyperv_util.h
# include openwsman.h
# define HYPERV_ERROR(code, ...) \
@@ -35,6 +36,7 @@
typedef struct _hypervPrivate hypervPrivate;
struct _hypervPrivate {
+hypervParsedUri *parsedUri;
WsManClient *client;
};
diff --git a/src/hyperv/hyperv_util.c b/src/hyperv/hyperv_util.c
new file mode 100644
index 000..298cfe0
--- /dev/null
+++ b/src/hyperv/hyperv_util.c
@@ -0,0 +1,129 @@
+
+/*
+ * hyperv_util.c: utility functions for the Microsoft Hyper-V driver
+ *
+ * Copyright (C) 2011 Matthias Bolte matthias.bo...@googlemail.com
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include config.h
+
+#include internal.h
+#include datatypes.h
+#include qparams.h
+#include util.h
+#include memory.h
+#include logging.h
+#include uuid.h
+#include hyperv_private.h
+#include hyperv_util.h
+
+#define VIR_FROM_THIS VIR_FROM_HYPERV
+
+
+
+int
+hypervParseUri(hypervParsedUri **parsedUri, xmlURIPtr uri)
+{
+int result = -1;
+struct qparam_set *queryParamSet = NULL;
+struct qparam *queryParam = NULL;
+int i;
+
+if (parsedUri == NULL || *parsedUri != NULL) {
+HYPERV_ERROR(VIR_ERR_INTERNAL_ERROR, %s, _(Invalid argument));
+return -1;
+}
+
+if (VIR_ALLOC(*parsedUri) 0) {
+virReportOOMError();
+return -1;
+}
+
+#ifdef HAVE_XMLURI_QUERY_RAW
+queryParamSet = qparam_query_parse(uri-query_raw);
+#else
+queryParamSet = qparam_query_parse(uri-query);
+#endif
+
+if (queryParamSet == NULL) {
+goto cleanup;
+}
+
+for (i = 0; i
[libvirt] [PATCHv2 0/5] Add basic driver for Microsoft Hyper-V
Version 2 of the Hyper-V driver. See the individual patches for the changes from version 1. Complete version 1 can be found here https://www.redhat.com/archives/libvir-list/2011-July/msg00766.html This series adds the basic driver and supports listing and retrieving information about existing domains and their lifecycle management. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/5] hyperv: Add driver skeleton
) \
+ -I@top_srcdir@/src/conf $(AM_CFLAGS)
+libvirt_driver_hyperv_la_LDFLAGS = $(AM_LDFLAGS)
+libvirt_driver_hyperv_la_LIBADD = $(OPENWSMAN_LIBS)
+if WITH_DRIVER_MODULES
+libvirt_driver_hyperv_la_LIBADD += ../gnulib/lib/libgnu.la
+libvirt_driver_hyperv_la_LDFLAGS += -module -avoid-version
+endif
+libvirt_driver_hyperv_la_SOURCES = $(HYPERV_DRIVER_SOURCES)
+endif
+
if WITH_NETWORK
if WITH_DRIVER_MODULES
mod_LTLIBRARIES += libvirt_driver_network.la
@@ -1000,6 +1028,7 @@ EXTRA_DIST +=
\
$(LIBXL_DRIVER_SOURCES) \
$(ESX_DRIVER_SOURCES) \
$(ESX_DRIVER_EXTRA_DIST)\
+ $(HYPERV_DRIVER_SOURCES)\
$(NETWORK_DRIVER_SOURCES) \
$(INTERFACE_DRIVER_SOURCES) \
$(STORAGE_DRIVER_SOURCES) \
diff --git a/src/driver.h b/src/driver.h
index 70ea4c2..589f232 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -29,6 +29,7 @@ typedef enum {
VIR_DRV_XENAPI = 12,
VIR_DRV_VMWARE = 13,
VIR_DRV_LIBXL = 14,
+VIR_DRV_HYPERV = 15,
} virDrvNo;
diff --git a/src/hyperv/hyperv_device_monitor.c
b/src/hyperv/hyperv_device_monitor.c
new file mode 100644
index 000..ea9e068
--- /dev/null
+++ b/src/hyperv/hyperv_device_monitor.c
@@ -0,0 +1,79 @@
+
+/*
+ * hyperv_device_monitor.c: device monitor functions for managing
+ * Microsoft Hyper-V host devices
+ *
+ * Copyright (C) 2011 Matthias Bolte matthias.bo...@googlemail.com
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include config.h
+
+#include internal.h
+#include virterror_internal.h
+#include datatypes.h
+#include util.h
+#include memory.h
+#include logging.h
+#include uuid.h
+#include hyperv_device_monitor.h
+
+#define VIR_FROM_THIS VIR_FROM_HYPERV
+
+
+
+static virDrvOpenStatus
+hypervDeviceOpen(virConnectPtr conn,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+
+if (conn-driver-no != VIR_DRV_HYPERV) {
+return VIR_DRV_OPEN_DECLINED;
+}
+
+conn-devMonPrivateData = conn-privateData;
+
+return VIR_DRV_OPEN_SUCCESS;
+}
+
+
+
+static int
+hypervDeviceClose(virConnectPtr conn)
+{
+conn-devMonPrivateData = NULL;
+
+return 0;
+}
+
+
+
+static virDeviceMonitor hypervDeviceMonitor = {
+Hyper-V,
+.open = hypervDeviceOpen, /* 0.9.5 */
+.close = hypervDeviceClose, /* 0.9.5 */
+};
+
+
+
+int
+hypervDeviceRegister(void)
+{
+return virRegisterDeviceMonitor(hypervDeviceMonitor);
+}
diff --git a/src/hyperv/hyperv_device_monitor.h
b/src/hyperv/hyperv_device_monitor.h
new file mode 100644
index 000..864e8af
--- /dev/null
+++ b/src/hyperv/hyperv_device_monitor.h
@@ -0,0 +1,29 @@
+
+/*
+ * hyperv_device_monitor.h: device monitor functions for managing
+ * Microsoft Hyper-V host devices
+ *
+ * Copyright (C) 2011 Matthias Bolte matthias.bo...@googlemail.com
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __HYPERV_DEVICE_MONITOR_H__
+# define __HYPERV_DEVICE_MONITOR_H__
+
+int hypervDeviceRegister(void);
+
+#endif /* __HYPERV_DEVICE_MONITOR_H__ */
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
new file mode 100644
index 000..eb01bac
--- /dev/null
+++ b/src/hyperv/hyperv_driver.c
@@ -0,0 +1,108
[libvirt] [PATCHv2 1/5] hyperv: Add configure check for OpenWSMAN
---
v2:
- relax OpenWSMAN requirement to 2.2.3
- move libvirt.spec.in change from 2/5 here and fix OpenWSMAN package name
- qoute all PKG_CHECK_MODULES parameters
configure.ac| 38 ++
libvirt.spec.in |9 +
2 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index e9d5be4..d7ebe79 100644
--- a/configure.ac
+++ b/configure.ac
@@ -66,6 +66,7 @@ XMLRPC_REQUIRED=1.14.0
HAL_REQUIRED=0.5.0
DEVMAPPER_REQUIRED=1.0.0
LIBCURL_REQUIRED=7.18.0
+OPENWSMAN_REQUIRED=2.2.3
LIBPCAP_REQUIRED=1.0.0
LIBNL_REQUIRED=1.1
LIBSSH2_REQUIRED=1.0
@@ -275,6 +276,8 @@ AC_ARG_WITH([lxc],
AC_HELP_STRING([--with-lxc], [add Linux Container support
@:@default=check@:@]),[],[with_lxc=check])
AC_ARG_WITH([esx],
AC_HELP_STRING([--with-esx], [add ESX support
@:@default=check@:@]),[],[with_esx=check])
+AC_ARG_WITH([hyperv],
+ AC_HELP_STRING([--with-hyperv], [add Hyper-V support
@:@default=check@:@]),[],[with_hyperv=check])
AC_ARG_WITH([test],
AC_HELP_STRING([--with-test], [add test driver support
@:@default=yes@:@]),[],[with_test=yes])
AC_ARG_WITH([remote],
@@ -1912,6 +1915,35 @@ LIBCURL_CFLAGS=-DCURL_DISABLE_TYPECHECK $LIBCURL_CFLAGS
AC_SUBST([LIBCURL_CFLAGS])
AC_SUBST([LIBCURL_LIBS])
+
+dnl
+dnl check for openwsman (Hyper-V)
+dnl
+
+OPENWSMAN_CFLAGS=
+OPENWSMAN_LIBS=
+
+if test $with_hyperv = yes || test $with_hyperv = check; then
+PKG_CHECK_MODULES([OPENWSMAN], [openwsman = $OPENWSMAN_REQUIRED], [
+if test $with_hyperv = check; then
+with_hyperv=yes
+fi
+], [
+if test $with_hyperv = check; then
+with_hyperv=no
+AC_MSG_NOTICE([openwsman is required for the Hyper-V driver,
disabling it])
+elif test $with_hyperv = yes; then
+AC_MSG_ERROR([openwsman = $OPENWSMAN_REQUIRED is required for the
Hyper-V driver])
+fi
+])
+fi
+
+if test $with_hyperv = yes ; then
+AC_DEFINE_UNQUOTED([WITH_HYPERV], 1, [whether Hyper-V driver is enabled])
+fi
+AM_CONDITIONAL([WITH_HYPERV], [test $with_hyperv = yes])
+
+
dnl
dnl check for python
dnl
@@ -2414,6 +2446,7 @@ AC_MSG_NOTICE([xenlight: $with_libxl])
AC_MSG_NOTICE([ LXC: $with_lxc])
AC_MSG_NOTICE([PHYP: $with_phyp])
AC_MSG_NOTICE([ ESX: $with_esx])
+AC_MSG_NOTICE([ Hyper-V: $with_hyperv])
AC_MSG_NOTICE([Test: $with_test])
AC_MSG_NOTICE([ Remote: $with_remote])
AC_MSG_NOTICE([ Network: $with_network])
@@ -2455,6 +2488,11 @@ AC_MSG_NOTICE([ libcurl: $LIBCURL_CFLAGS $LIBCURL_LIBS])
else
AC_MSG_NOTICE([ libcurl: no])
fi
+if test $with_hyperv = yes ; then
+AC_MSG_NOTICE([openwsman: $OPENWSMAN_CFLAGS $OPENWSMAN_LIBS])
+else
+AC_MSG_NOTICE([openwsman: no])
+fi
if test $with_libssh2 != no ; then
AC_MSG_NOTICE([ libssh2: $LIBSSH2_CFLAGS $LIBSSH2_LIBS])
else
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 230237e..c971681 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -50,6 +50,7 @@
# Then the hypervisor drivers that talk a native remote protocol
%define with_phyp 0%{!?_without_phyp:1}
%define with_esx 0%{!?_without_esx:1}
+%define with_hyperv0%{!?_without_hyperv:1}
%define with_xenapi0%{!?_without_xenapi:1}
# Then the secondary host drivers
@@ -437,6 +438,9 @@ BuildRequires: libcurl-devel
BuildRequires: curl-devel
%endif
%endif
+%if %{with_hyperv}
+BuildRequires: libwsman-devel = 2.2.3
+%endif
%if %{with_audit}
BuildRequires: audit-libs-devel
%endif
@@ -569,6 +573,10 @@ of recent versions of Linux (and other OSes).
%define _without_esx --without-esx
%endif
+%if ! %{with_hyperv}
+%define _without_hyperv --without-hyperv
+%endif
+
%if ! %{with_vmware}
%define _without_vmware --without-vmware
%endif
@@ -687,6 +695,7 @@ of recent versions of Linux (and other OSes).
%{?_without_one} \
%{?_without_phyp} \
%{?_without_esx} \
+ %{?_without_hyperv} \
%{?_without_vmware} \
%{?_without_network} \
%{?_with_rhel5_api} \
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix detection of GnuTLS 1.x.y
2011/8/3 Eric Blake ebl...@redhat.com: On 08/03/2011 08:22 AM, Matthias Bolte wrote: Detection based on gnutls_session doesn't work because GnuTLS 2.x.y comes with a compat.h that defines gnutls_session to gnutls_session_t. Instead detect this based on LIBGNUTLS_VERSION_MAJOR. --- configure.ac | 22 +++--- 1 files changed, 15 insertions(+), 7 deletions(-) ACK for correctness. However, I wonder if it is the most efficient. We aren't using this as a Makefile conditional, and LIBGNUTLS_VERSION_MAJOR is available at compile time. That is, I think that we could skip the configure check altogether, and just move the #if LIBGNUTLS_VERSION_MAJOR 2 check into a common header used by all files that want to use gnutls in the first place, for a slightly smaller and faster configure. Here's a v2 that does this. -- Matthias Bolte http://photron.blogspot.com From 08d8e6a4413fd8bb562d42a7bc242806ed5a Mon Sep 17 00:00:00 2001 From: Matthias Bolte matthias.bo...@googlemail.com Date: Wed, 3 Aug 2011 18:23:21 +0200 Subject: [PATCH] Fix detection of GnuTLS 1.x.y Detection based on gnutls_session doesn't work because GnuTLS 2.x.y comes with a compat.h that defines gnutls_session to gnutls_session_t. Instead detect this based on LIBGNUTLS_VERSION_MAJOR. Move this from configure/config.h to gnutls_1_0_compat.h and make sure that all users include gnutls_1_0_compat.h properly. Also fix header guard in gnutls_1_0_compat.h. --- configure.ac | 14 -- src/gnutls_1_0_compat.h |8 tests/virnettlscontexttest.c |6 +++--- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/configure.ac b/configure.ac index c8d291b..fe700b9 100644 --- a/configure.ac +++ b/configure.ac @@ -834,20 +834,6 @@ fi AC_SUBST([GNUTLS_CFLAGS]) AC_SUBST([GNUTLS_LIBS]) -dnl Old versions of GnuTLS uses types like 'gnutls_session' instead -dnl of 'gnutls_session_t'. Try to detect this type if defined so -dnl that we can offer backwards compatibility. -old_cflags=$CFLAGS -old_libs=$LIBS -CFLAGS=$CFLAGS $GNUTLS_CFLAGS -LIBS=$LIBS $GNUTLS_LIBS -AC_CHECK_TYPE([gnutls_session], - AC_DEFINE([GNUTLS_1_0_COMPAT],[], - [enable GnuTLS 1.0 compatibility macros]),, - [#include gnutls/gnutls.h]) -CFLAGS=$old_cflags -LIBS=$old_libs - dnl Cyrus SASL AC_ARG_WITH([sasl], diff --git a/src/gnutls_1_0_compat.h b/src/gnutls_1_0_compat.h index fb423f1..fd9cc1c 100644 --- a/src/gnutls_1_0_compat.h +++ b/src/gnutls_1_0_compat.h @@ -21,9 +21,17 @@ */ #ifndef LIBVIRT_GNUTLS_1_0_COMPAT_H__ +# define LIBVIRT_GNUTLS_1_0_COMPAT_H__ # include config.h +# include gnutls/gnutls.h + +/* enable backward compatibility macros for gnutls 1.x.y */ +# if LIBGNUTLS_VERSION_MAJOR 2 +# define GNUTLS_1_0_COMPAT +# endif + # ifdef GNUTLS_1_0_COMPAT # define gnutls_session_t gnutls_session # define gnutls_x509_crt_tgnutls_x509_crt diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 520b006..ee7b6b4 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -23,6 +23,8 @@ #include stdlib.h #include fcntl.h #include sys/socket.h +#include gnutls/gnutls.h +#include gnutls/x509.h #include testutils.h #include util.h @@ -32,12 +34,10 @@ #include virfile.h #include command.h #include network.h +#include gnutls_1_0_compat.h #if !defined WIN32 HAVE_LIBTASN1_H !defined GNUTLS_1_0_COMPAT # include libtasn1.h -# include gnutls/gnutls.h -# include gnutls/x509.h -# include gnutls_1_0_compat.h # include rpc/virnettlscontext.h -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix detection of GnuTLS 1.x.y
2011/8/3 Eric Blake ebl...@redhat.com: On 08/03/2011 10:28 AM, Matthias Bolte wrote: 2011/8/3 Eric Blakeebl...@redhat.com: On 08/03/2011 08:22 AM, Matthias Bolte wrote: Detection based on gnutls_session doesn't work because GnuTLS 2.x.y comes with a compat.h that defines gnutls_session to gnutls_session_t. Instead detect this based on LIBGNUTLS_VERSION_MAJOR. --- configure.ac | 22 +++--- 1 files changed, 15 insertions(+), 7 deletions(-) ACK for correctness. However, I wonder if it is the most efficient. We aren't using this as a Makefile conditional, and LIBGNUTLS_VERSION_MAJOR is available at compile time. That is, I think that we could skip the configure check altogether, and just move the #if LIBGNUTLS_VERSION_MAJOR 2 check into a common header used by all files that want to use gnutls in the first place, for a slightly smaller and faster configure. Here's a v2 that does this. Nicer - thanks for that work. Detection based on gnutls_session doesn't work because GnuTLS 2.x.y comes with a compat.h that defines gnutls_session to gnutls_session_t. Instead detect this based on LIBGNUTLS_VERSION_MAJOR. Move this from configure/config.h to gnutls_1_0_compat.h and make sure that all users include gnutls_1_0_compat.h properly. Also fix header guard in gnutls_1_0_compat.h. ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: update to latest gnulib
2011/8/3 Eric Blake ebl...@redhat.com: I noticed that with 0.9.4, gnulib ended up replacing pthread_sigmask on glibc, even though glibc's works perfectly fine. It turns out to have been an upstream gnulib bug. * .gnulib: Update to latest, for pthread_sigmask fix. --- This missed the 0.9.4 release; oh well. * .gnulib 41a7841...7f494c7 (18): pthread_sigmask: Actually use results of gl_THREADLIB. autoupdate maint.mk: relax the default _gl_TS_function_match regexp git-version-gen: document that EXTRA_DIST must include .version wctype-h: Fix last change. frexpl: Update autoconf test. sys_utsname: Add support for Minix. strings: Add support for Minix. wctype-h: Add support for Minix. * lib/xalloc.h (DEFAULT_MXFAST): Track 64-bit glibc. stdioext: Add support for Minix. errno: Port to Minix. Work around declaration collisions on Minix. Add support for Minix with ACK compiler. Documentation about Minix. snippet/warn-on-use: Fix indentation. tests: test-update-copyright.sh: remove unnecessary rm commands maint.mk: avoid sc_prohibit_always-defined_macros failure in coreutils .gnulib | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/.gnulib b/.gnulib index 41a7841..7f494c7 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 41a7841a82351f9df3e13b6199a134004d694131 +Subproject commit 7f494c7d725db4a7f3abdef09d4070725487da67 -- 1.7.4.4 ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
2011/8/2 Daniel P. Berrange berra...@redhat.com: On Thu, Jul 28, 2011 at 07:56:18AM -0600, Eric Blake wrote: On 07/28/2011 07:52 AM, Matthias Bolte wrote: At least all tests compile on FreeBSD again. But most of the SSH cases in virnetmessagetest are failing and I don't understand why yet. Could it be a PATH vs. exec() issue, where BSD ends up doing a slightly different PATH search and not executing the dummy 'ssh' script from our test directory? Does a ktrace (or truss or strace or however it's spelled) shed any light? NB, if you run ./virnetsockettest it won't work. You have to run PATH=`pwd`:$PATH ./virnetsockettest That's true and this way is works on FreeBSD too. Or make check TESTS=virnetsockettest to ensure the $PATH is set to find the local fake ssh This way it doesn't because the makefile extends the path in a non-portable way. Eric fixed this http://libvirt.org/git/?p=libvirt.git;a=commit;h=343ab98229a60126ec75087dd425207392b77754 -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Don't leak the virtual port profile in virNetworkDefFree
Reported by Alex Jia. --- src/conf/network_conf.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6714c20..b11c482 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -169,6 +169,8 @@ void virNetworkDefFree(virNetworkDefPtr def) virNetworkDNSDefFree(def-dns); +VIR_FREE(def-virtPortProfile); + virBandwidthDefFree(def-bandwidth); VIR_FREE(def); -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of 0.9.4 candidate release 2 rc1
2011/8/1 Alex Jia a...@redhat.com: On 07/30/2011 03:02 PM, Daniel Veillard wrote: I actually tagged and pushed the rc2 tarball and rpms yesterday but completely forgot to send the associated mail, oops ! ftp://libvirt.org/libvirt/libvirt-0.9.4-rc2.tar.gz Hopefully it fixes most of the problems raised with rc1, including a number of leaks. Please report and if you had an issue with rc1 which is still not fixed there (or in git) please raise it ASAP. I'm planning for the final release early Tuesday 2 morning (i.e. late Monday for most :-) Daniel Hi Daniel, I met some memory leaks and invalid read error when I run 'make -C tests valgrind', they may be test cases itself issue, the attachment is a log, please check it. The invalid reads by __strspn_sse42 and __strspn_sse42 are actually bugs in glibc where it uses optimized code on too short strings. Nothing we could fix from the libvirt side. I've send a patch form the memory leak https://www.redhat.com/archives/libvir-list/2011-August/msg3.html -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Don't leak the virtual port profile in virNetworkDefFree
2011/8/1 Osier Yang jy...@redhat.com: 于 2011年08月01日 16:07, Matthias Bolte 写道: Reported by Alex Jia. --- src/conf/network_conf.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6714c20..b11c482 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -169,6 +169,8 @@ void virNetworkDefFree(virNetworkDefPtr def) virNetworkDNSDefFree(def-dns); + VIR_FREE(def-virtPortProfile); + virBandwidthDefFree(def-bandwidth); VIR_FREE(def); ACK Osier Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of 0.9.4 candidate release 2 rc1
2011/8/1 Wen Congyang we...@cn.fujitsu.com: At 07/30/2011 03:02 PM, Daniel Veillard Write: I actually tagged and pushed the rc2 tarball and rpms yesterday but completely forgot to send the associated mail, oops ! ftp://libvirt.org/libvirt/libvirt-0.9.4-rc2.tar.gz Hopefully it fixes most of the problems raised with rc1, including a number of leaks. Please report and if you had an issue with rc1 which is still not fixed there (or in git) please raise it ASAP. I'm planning for the final release early Tuesday 2 morning (i.e. late Monday for most :-) If client(for example: virsh) exits unexpectedly, it will cause libvirtd crashed. Steps to reproduce this problem(vm1 does not run): 1. for ((i=0; i 50; i++)); do virsh managedsave vm1 done; killall virsh The reason is that we free virNetServerClient when the refs is not 0. I'm not sure what you mean here. virNetClientFree frees the client when the last ref is removed. I read the code under the directory src/rpc/, and find we have xxxRef(), but we do not have xxxUnref(). And sometimes we free the data structure if ref is not 0. We add an reference of the data structure, but sometimes we forget to unref it. We already have an unref function it's called virNetClientFree. Are you referring to virNetClientClose that closes the connection and frees parts of the client? I tried to figure out the cleanup and ref counting logic in the RPC code in order to fix a memory leak triggered by virsh. https://www.redhat.com/archives/libvir-list/2011-July/msg02011.html It's complex and I don't understand it completely, as there is ref counting on multiple levels that is entangled with the event loop. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix a regression of domjobabort
2011/8/1 Osier Yang jy...@redhat.com:
Introduced by f9a837da73a11ef, the condition is not changed after
the else clause is removed. So now it quit with domain is not
running when the domain is running. However, when the domain is
not running, it reports no job is active.
How to reproduce:
1)
% virsh start $domain
% virsh domjobabort $domain
error: Requested operation is not valid: domain is not running
2)
% virsh destroy $domain
% virsh domjobabort $domain
error: Requested operation is not valid: no job is active on the domain
3)
% virsh save $domain /tmp/$domain.save
Before above commands finished, try to abort job in another terminal
% virsh domabortjob $domain
error: Requested operation is not valid: domain is not running
---
src/qemu/qemu_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b673fd5..cce1c68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8065,7 +8065,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_ABORT) 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
+ if (!virDomainObjIsActive(vm)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
goto endjob;
ACK, makes condition and error message match again.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Fix vol-name and vol-pool commands
This commands don't have a --pool option, so don't tell
vshCommandOptVolBy that there could be one. This made
vshCommandOptString for pooloptname fail and an missing option
error was reported.
Make pooloptname optional for vshCommandOptVolBy.
---
tools/virsh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 9e0744d..5fcf370 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9201,7 +9201,7 @@ cmdVolName(vshControl *ctl, const vshCmd *cmd)
if (!vshConnectionUsability(ctl, ctl-conn))
return false;
-if (!(vol = vshCommandOptVolBy(ctl, cmd, vol, pool, NULL,
+if (!(vol = vshCommandOptVolBy(ctl, cmd, vol, NULL, NULL,
VSH_BYUUID)))
return false;
@@ -9238,7 +9238,7 @@ cmdVolPool(vshControl *ctl, const vshCmd *cmd)
return false;
/* Use the supplied string to locate the volume */
-if (!(vol = vshCommandOptVolBy(ctl, cmd, vol, pool, NULL,
+if (!(vol = vshCommandOptVolBy(ctl, cmd, vol, NULL, NULL,
VSH_BYUUID))) {
return false;
}
@@ -13619,7 +13619,7 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
if (vshCommandOptString(cmd, optname, n) = 0)
return NULL;
-if (vshCommandOptString(cmd, pooloptname, p) 0) {
+if (pooloptname != NULL vshCommandOptString(cmd, pooloptname, p) 0) {
vshError(ctl, %s, _(missing option));
return NULL;
}
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Fix vol-name and vol-pool commands
2011/8/1 Daniel Veillard veill...@redhat.com: On Mon, Aug 01, 2011 at 02:43:25PM +0200, Matthias Bolte wrote: This commands don't have a --pool option, so don't tell vshCommandOptVolBy that there could be one. This made vshCommandOptString for pooloptname fail and an missing option error was reported. Make pooloptname optional for vshCommandOptVolBy. oops replied privately, ACK thanks ! Daniel Pushed, now. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of 0.9.4 candidate release 2 rc1
2011/8/1 Wen Congyang wencongy...@gmail.com: At 08/01/2011 06:02 PM, Matthias Bolte write: 2011/8/1 Wen Congyangwe...@cn.fujitsu.com: At 07/30/2011 03:02 PM, Daniel Veillard Write: I actually tagged and pushed the rc2 tarball and rpms yesterday but completely forgot to send the associated mail, oops ! ftp://libvirt.org/libvirt/libvirt-0.9.4-rc2.tar.gz Hopefully it fixes most of the problems raised with rc1, including a number of leaks. Please report and if you had an issue with rc1 which is still not fixed there (or in git) please raise it ASAP. I'm planning for the final release early Tuesday 2 morning (i.e. late Monday for most :-) If client(for example: virsh) exits unexpectedly, it will cause libvirtd crashed. Steps to reproduce this problem(vm1 does not run): 1. for ((i=0; i 50; i++)); do virsh managedsave vm1 done; killall virsh The reason is that we free virNetServerClient when the refs is not 0. I'm not sure what you mean here. virNetClientFree frees the client when the last ref is removed. I read the code under the directory src/rpc/, and find we have xxxRef(), but we do not have xxxUnref(). And sometimes we free the data structure if ref is not 0. We add an reference of the data structure, but sometimes we forget to unref it. We already have an unref function it's called virNetClientFree. Sorry for confusing you. The reason is that: In the function virNetServerClientClose(), we set client-sock to NULL while we still use it. Thanks. Wen Congyang Ah, you're talking about the server side. I only looked at the client side of the RPC code while trying to understand the memleak. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virNetClientPtr leak in remote driver
2011/8/1 Eric Blake ebl...@redhat.com: On 07/28/2011 12:07 PM, Matthias Bolte wrote: 2011/7/27 Matthias Bolte matthias.bo...@googlemail.com: doRemoteClose doesn't free the virNetClientPtr and this creates a 260kb leak per remote connection. This happens because virNetClientFree doesn't remove the last ref, because virNetClientNew creates the virNetClientPtr with a refcount of 2. The memory leak I saw was due to virsh calling virEventRegisterDefaultImpl, but not calling virEventRunDefaultImpl. Because the event loop is initialized, the call to virNetSocketAddIOCallback in virNetClientNew succeeds. But virsh not driving the event loop results in not removing callbacks that were marked for deletion. Finally this leaks the virNetClientPtr using in the remote driver. I used a virsh -c qemu:///system to test. I was able fix this by calling virEventRunDefaultImpl after virConnectClose in virsh. But I don't think that this is the correct general solution. Where do we stand with 0.9.4? Is this something where we need the general fix before the release, or is just the virsh hack to call virEventRunDefaultImpl good enough for the release, or is this problem not severe enough and we can release as-is? virsh is a bit special here as it registers/initializes the default even loop but doesn't drive it properly. It only drives it for the console command. That's the problem in virsh. This can be avoided by calling virEventRunDefaultImpl after virConnectClose iff it's a remote connection, in other cases virEventRunDefaultImpl might block. Therefore, we shouldn't be calling it in general after a virConnectClose in virsh. If we assume that an application that registers an event loop will drive it properly then this problem is not critical, as it doesn't exist. Just virsh is a special case that leaks 260kb per closed remote connection. When we assume that a typical virsh user uses a single connection per virsh invocation then we can view this as a static leak. Therefore, I'd say we could just leave this as-is for 0.9.4, except Dan would be back soon and the problem is easy to fix for him. Another problem is the libvirtd crash that Wen discovered. That's more serious, as a misbehaving client shouldn't be able to crash libvirtd. https://www.redhat.com/archives/libvir-list/2011-August/msg5.html -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] compile error: cannot open gnulib/lib/gnulib.mk: No such file or directory
2011/7/30 ajia a...@redhat.com:
On 07/30/2011 10:30 PM, ajia wrote:
Hi all,
When I get latest libvirt and then runnning make make install, I met
the following error:
# make make install
INFO: gnulib update required; running ./autogen.sh first
CDPATH=${ZSH_VERSION+.}: cd . /bin/sh
/root/libvirt/build-aux/missing --run aclocal-1.11 -I m4 -I gnulib/m4
cd . /bin/sh /root/libvirt/build-aux/missing --run automake-1.11 --gnu
automake-1.11: cannot open gnulib/lib/gnulib.mk: No such file or
directory
make: *** [Makefile.in] Error 1
Has anybody met this issue recently?
BTW, my current commit id is a8be259d0cca071c5735c70580c247e9b44b2ebd.
Thanks,
Alex
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This issue is introduced by b590866 freebsd: Fix build problem due to
picking up the wrong libvirt.h,
please correct me if I'm wrong, thanks.
Correct, rerunning ./bootstrap should fix this.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Avoid /bin/true in commandtest
2011/7/28 Eric Blake ebl...@redhat.com: On 07/28/2011 09:52 AM, Matthias Bolte wrote: Rely on PATH and use just true, because on FreeBSD it's /usr/bin/true. What fun. The autoconf manual has this gem under true, apropos to the current patch: | when asked whether false is more portable than true Alexandre Oliva answered: | | In a sense, yes, because if it doesn't exist, the shell will produce an exit status of failure, which is correct for false, but not for true. http://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins --- tests/commanddata/test16.log | 2 +- tests/commandtest.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Don't use bash if we don't have to
This tested failed on FreeBSD because it was using bash, that might not be installed. --- tests/int-overflow |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/int-overflow b/tests/int-overflow index baf2eef..36e5536 100755 --- a/tests/int-overflow +++ b/tests/int-overflow @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh # Ensure that an invalid domain ID isn't interpreted as a valid one. # Before, an ID of 2^32+2 would be treated just like an ID of 2. -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] freebsd: Fix build problem due to picking up the wrong libvirt.h
2011/7/28 Eric Blake ebl...@redhat.com: From: Matthias Bolte matthias.bo...@googlemail.com Gettext annoyingly modifies CPPFLAGS in-place, putting -I/usr/local/include into the search patch if libintl headers must be used from that location. But since we must support automake 1.9.6 which lacks AM_CPPFLAGS, and since CPPFLAGS is used prior to INCLUDES, this means that the build picks up the _old_ installed libvirt.h in priority to the in-tree version, leading to all sorts of weird build failures on FreeBSD. Fix this by teaching configure to undo gettext's actions, but to keep any changes required by gettext at the end of INCLUDES after all in-tree locations are used first. Also requires adding a wrapper Makefile.am and making gnulib-tool create just gnulib.mk files during the bootstrap process. --- v1 is here: https://www.redhat.com/archives/libvir-list/2011-July/msg01984.html v2: update bootstrap.conf and gnulib/*/Makefile.am to fix gnulib compilation, update .gitignore to allow committing new files. Works. ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Don't use bash if we don't have to
2011/7/29 Eric Blake ebl...@redhat.com: On 07/29/2011 06:19 AM, Matthias Bolte wrote: This tested failed on FreeBSD because it was using bash, that might not be installed. --- tests/int-overflow | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/int-overflow b/tests/int-overflow index baf2eef..36e5536 100755 --- a/tests/int-overflow +++ b/tests/int-overflow @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh This script sources test-lib.sh, which in turn uses features like $() that are not portable to Solaris /bin/sh. But I didn't see any bash-isms - everything in those two files appears to be safe for use with POSIX sh, so I'm okay with the patch as is: ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid non-portable shell in test setup
2011/7/29 Eric Blake ebl...@redhat.com:
POSIX states that 'a=1; a=2 b=$a command' has unspecified results
for the value of $b visible within command. In particular, on
BSD, this resulted in PATH not picking up the in-test ssh.
* tests/Makefile.am (lv_abs_top_builddir): New macro.
(path_add, TESTS_ENVIRONMENT): Use it to avoid referring to an
environment variable set previously within the same command line.
Reported by Matthias Bolte.
---
Spotted by inspection based on an IRC report; hopefully someone
on BSD can test if it actually makes a difference.
tests/Makefile.am | 10 +++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 43a4301..f4afcb9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -259,13 +259,17 @@ TESTS += interfacexml2xmltest
TESTS += cputest
-path_add =
$$abs_top_builddir/daemon$(PATH_SEPARATOR)$$abs_top_builddir/tools$(PATH_SEPARATOR)$$abs_top_builddir/tests
-
# NB, automake 1.10 does not provide the real
# abs_top_{src/build}dir or builddir variables, so don't rely
# on them here. Fake them with 'pwd'
+# Also, BSD sh doesn't like 'a=b b=$$a', so we can't use an
+# intermediate shell variable, but must do all the expansion in make
+
+lv_abs_top_builddir=`cd '$(top_builddir)'; pwd`
+path_add =
$(lv_abs_top_builddir)/daemon$(PATH_SEPARATOR)$(lv_abs_top_builddir)/tools$(PATH_SEPARATOR)$(lv_abs_top_builddir)/tests
+
TESTS_ENVIRONMENT = \
- abs_top_builddir=`cd '$(top_builddir)'; pwd` \
+ abs_top_builddir=$(lv_abs_top_builddir) \
abs_top_srcdir=`cd '$(top_srcdir)'; pwd` \
abs_builddir=`pwd` \
abs_srcdir=`cd '$(srcdir)'; pwd` \
--
1.7.4.4
ACK, this make gmake check pass completely on FreeBSD.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Start of freeze for libvirt-0.9.4 and availability of rc1
2011/7/26 Jason Helfman jhelf...@e-e.com: On Tue, Jul 26, 2011 at 12:26:20PM -0600, Eric Blake thus spake: On 07/26/2011 12:16 PM, Jason Helfman wrote: remote.c: At top level: remote.c:409: error: negative width in bit-field '_gl_verify_error_if_negative' remote.c: In function 'remoteDispatchDomainGetBlockJobInfo': remote.c:1630: error: 'virDomainBlockJobInfo' undeclared (first use in this function) Ah. You're running into the same problem that has been previously reported of compiling against the stale installed libvirt.h instead of the just-built in-tree libvirt.h. Matthias had started a patch for that, but it never got finished. https://www.redhat.com/archives/libvir-list/2011-May/msg01926.html I de-installed the port, and then continued with the make process, and it installed just fine. What is the status of this patch? If this isn't going to make it into the release, I can warn users that this port needs to be de-installed prior to building the port. It's fixed in git now http://libvirt.org/git/?p=libvirt.git;a=commit;h=b590866bdb0aea20eda5b96883b8744fedbba88d But it missed 0.9.4 RC2, so depending on how Daniel plans to do the release of 0.9.4 it might not be included. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid type-punning compiler warning
2011/7/27 Eric Blake ebl...@redhat.com: On RHEL 5, with gcc 4.1.2: rpc/virnetsaslcontext.c: In function 'virNetSASLSessionUpdateBufSize': rpc/virnetsaslcontext.c:396: warning: dereferncing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] * src/rpc/virnetsaslcontext.c (virNetSASLSessionUpdateBufSize): Use a union to work around gcc warning. --- src/rpc/virnetsaslcontext.c | 11 +++ 1 files changed, 7 insertions(+), 4 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
--- bootstrap.conf |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index f006a47..3b105b1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ configmake count-one-bits crypto/md5 dirname-lgpl +environ fclose fcntl-h ffs -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h
2011/7/26 Eric Blake ebl...@redhat.com: On 07/26/2011 02:45 PM, Matthias Bolte wrote: +++ b/configure.ac @@ -2011,8 +2011,16 @@ dnl Enable building libvirtd? AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd = xyes]) dnl Check for gettext - don't go any newer than what RHEL 5 supports +dnl +dnl save and restore CPPFLAGS around gettext check as the internal iconv +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting +dnl in the build picking up previously installed libvirt/libvirt.h instead +dnl of the correct one from the soucre tree + +save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) +CPPFLAGS=$save_CPPFLAGS But I'm still worried that we have to use -I/usr/local/include somewhere in the command line, which means we would have to modify src/Makefile.am (and friends) to have: INCLUDES= ... $(GETTEXT_CPPFLAGS) where GETTEXT_CPPFLAGS is substituted with the difference in $save_CPPFLAGS and $CPPFLAGS prior to the point where we restore $CPPFLAGS. That's probably what we should do here. Now how to compute this difference? When we assume that save_CPPFLAGS and CPPFLAGS have a common prefix that we need to strip to get GETTEXT_CPPFLAGS then we could do it like this echo $(CPPFLAGS) | cut -c 1-`expr length $(save_CPPFLAGS)` --complement - This is probably neither the best nor a robustest way to do this. Do you have a better idea? Yep (although I haven't tested it thoroughly): save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) GETTEXT_CPPFLAGS= if test x$save_CPPFLAGS != x$CPPFLAGS; then set dummy $CPPFLAGS for var do case $var in $save_CPPFLAGS ) ;; *) GETTEXT_CPPFLAGS=$GETTEXT_CPPFLAGS $var ;; esac done fi CPPFLAGS=$save_CPPFLAGS Okay, this works for libvirt itself, but it fails for make check as the global CPPFLAGS also affect gnulib, but with this patch it doesn't contain the gettext related parts anymore and the gnulib tests fail to find libintl.h because of this. Making check in gnulib/tests Making check in . In file included from wait-process.c:37: ../../gnulib/lib/gettext.h:28:22: error: libintl.h: No such file or directory Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this? I attached a preliminary patch. -- Matthias Bolte http://photron.blogspot.com diff --git a/configure.ac b/configure.ac index ac34efc..b61c8e4 100644 --- a/configure.ac +++ b/configure.ac @@ -2055,8 +2055,30 @@ dnl Enable building libvirtd? AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd = xyes]) dnl Check for gettext - don't go any newer than what RHEL 5 supports +dnl +dnl save and restore CPPFLAGS around gettext check as the internal iconv +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting +dnl in the build picking up previously installed libvirt/libvirt.h instead +dnl of the correct one from the soucre tree. +dnl compute the difference between save_CPPFLAGS and CPPFLAGS and append it +dnl to INCLUDES in order to preserve changes made by gettext but in a place +dnl that does not break the build +save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) +GETTEXT_CPPFLAGS= +if test x$save_CPPFLAGS != x$CPPFLAGS; then + set dummy $CPPFLAGS; shift + for var + do + case $var in + $save_CPPFLAGS ) ;; + *) GETTEXT_CPPFLAGS=$GETTEXT_CPPFLAGS $var ;; + esac + done +fi +CPPFLAGS=$save_CPPFLAGS +AC_SUBST([GETTEXT_CPPFLAGS]) ALL_LINGUAS=`cd $srcdir/po /dev/null ls *.po | sed 's+\.po$++'` diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 63c731e..d81c29c 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,5 +1,15 @@ ## Process this file with automake to produce Makefile.in +INCLUDES = \ + -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ + -I$(top_srcdir)/include -I$(top_builddir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/src/util \ + -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/rpc \ + -I$(top_srcdir)/src/remote \ + $(GETTEXT_CPPFLAGS) + CLEANFILES = DAEMON_GENERATED = \ @@ -79,13 +89,6 @@ libvirtd_SOURCES = $(DAEMON_SOURCES) #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L libvirtd_CFLAGS = \ - -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ - -I$(top_srcdir)/include -I$(top_builddir)/include \ - -I$(top_srcdir)/src \ - -I$(top_srcdir)/src/util \ - -I$(top_srcdir)/src/conf \ - -I$(top_srcdir)/src/rpc \ - -I$(top_srcdir)/src/remote \ $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ $(XDR_CFLAGS) $(POLKIT_CFLAGS) \ $(WARN_CFLAGS) \ diff --git a/python/Makefile.am b/python/Makefile.am index 51f005d..063b1bf 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -6,7 +6,8 @@ INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ -I$(top_builddir)/include \ - -I$(top_builddir)/$(subdir) + -I$(top_builddir
Re: [libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
2011/7/28 Eric Blake ebl...@redhat.com: On 07/28/2011 05:08 AM, Matthias Bolte wrote: --- bootstrap.conf | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index f006a47..3b105b1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ configmake count-one-bits crypto/md5 dirname-lgpl +environ ACK. POSIX is clear that 'environ' is not declared by any standard header unless you use vendor extensions, whereas the gnulib module 'environ' guarantees the declaration in unistd.h to match the glibc extension when _GNU_SOURCE is defined (that is, the gnulib module is the vendor extension that we need to get the declaration visible on BSD). At least all tests compile on FreeBSD again. But most of the SSH cases in virnetmessagetest are failing and I don't understand why yet. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] freebsd: Avoid /bin/true in commandtest
Rely on PATH and use just true, because on FreeBSD it's /usr/bin/true.
---
tests/commanddata/test16.log |2 +-
tests/commandtest.c |6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/commanddata/test16.log b/tests/commanddata/test16.log
index 2433a4d..7088165 100644
--- a/tests/commanddata/test16.log
+++ b/tests/commanddata/test16.log
@@ -1 +1 @@
-A=B /bin/true C
+A=B true C
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 2e800ae..9ba53b8 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -566,9 +566,9 @@ cleanup:
*/
static int test16(const void *unused ATTRIBUTE_UNUSED)
{
-virCommandPtr cmd = virCommandNew(/bin/true);
+virCommandPtr cmd = virCommandNew(true);
char *outactual = NULL;
-const char *outexpect = A=B /bin/true C;
+const char *outexpect = A=B true C;
int ret = -1;
int fd = -1;
@@ -610,7 +610,7 @@ cleanup:
*/
static int test17(const void *unused ATTRIBUTE_UNUSED)
{
-virCommandPtr cmd = virCommandNew(/bin/true);
+virCommandPtr cmd = virCommandNew(true);
int ret = -1;
char *outbuf;
char *errbuf;
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Unify style of test skipping code
Prefer 'return EXIT_AM_SKIP' over 'exit(EXIT_AM_SKIP)'.
Prefer 'int main(void)' over 'int main(int argc, char **argv)'.
Fix mymain signature in commandtest and nodeinfotest.
---
tests/commandtest.c | 10 +-
tests/nodeinfotest.c | 10 +-
tests/qemuargv2xmltest.c |6 +-
tests/qemuxml2xmltest.c |6 +-
tests/virnettlscontexttest.c |6 --
tests/virshtest.c| 26 +-
6 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index ef2850d..2e800ae 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -38,10 +38,10 @@
#ifdef WIN32
-static int
-mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+int
+main(void)
{
-exit (EXIT_AM_SKIP);
+return EXIT_AM_SKIP;
}
#else
@@ -814,6 +814,6 @@ mymain(void)
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
-#endif /* !WIN32 */
-
VIRT_TEST_MAIN(mymain)
+
+#endif /* !WIN32 */
diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index 5abee92..448e072 100644
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -15,10 +15,10 @@
defined(__amd64__) || \
defined(__i386__)))
-static int
-mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+int
+main(void)
{
-exit (EXIT_AM_SKIP);
+return EXIT_AM_SKIP;
}
#else
@@ -130,6 +130,6 @@ mymain(void)
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
-#endif /* __linux__ */
-
VIRT_TEST_MAIN(mymain)
+
+#endif /* __linux__ */
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index bade95d..c2b6cf2 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -239,6 +239,10 @@ VIRT_TEST_MAIN(mymain)
#else
-int main (void) { return (EXIT_AM_SKIP); }
+int
+main(void)
+{
+return EXIT_AM_SKIP;
+}
#endif /* WITH_QEMU */
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index f1900c5..461fd0d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -204,6 +204,10 @@ VIRT_TEST_MAIN(mymain)
#else
-int main (void) { exit (EXIT_AM_SKIP); }
+int
+main(void)
+{
+return EXIT_AM_SKIP;
+}
#endif /* WITH_QEMU */
diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c
index 12ecf1e..520b006 100644
--- a/tests/virnettlscontexttest.c
+++ b/tests/virnettlscontexttest.c
@@ -1249,9 +1249,11 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
+
int
-main(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+main(void)
{
-exit (EXIT_AM_SKIP);
+return EXIT_AM_SKIP;
}
+
#endif
diff --git a/tests/virshtest.c b/tests/virshtest.c
index e22e582..de5138c 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -9,7 +9,17 @@
#include util.h
#include testutils.h
-#define DOM_UUID ef861801-45b9-11cb-88e3-afbfe5370493
+#ifdef WIN32
+
+int
+main(void)
+{
+return EXIT_AM_SKIP;
+}
+
+#else
+
+# define DOM_UUID ef861801-45b9-11cb-88e3-afbfe5370493
static const char *dominfo_fc4 = \
Id: 2\n\
@@ -71,13 +81,13 @@ cleanup:
return result;
}
-#define VIRSH_DEFAULT ../tools/virsh, \
+# define VIRSH_DEFAULT ../tools/virsh, \
--connect, \
test:///default
static char *custom_uri;
-#define VIRSH_CUSTOM ../tools/virsh, \
+# define VIRSH_CUSTOM ../tools/virsh, \
--connect, \
custom_uri
@@ -224,10 +234,6 @@ mymain(void)
{
int ret = 0;
-#ifdef WIN32
-exit (EXIT_AM_SKIP);
-#endif
-
if (virAsprintf(custom_uri, test://%s/../examples/xml/test/testnode.xml,
abs_srcdir) 0)
return EXIT_FAILURE;
@@ -298,7 +304,7 @@ mymain(void)
/* It's a bit awkward listing result before argument, but that's a
* limitation of C99 vararg macros. */
-#define DO_TEST(i, result, ...) \
+# define DO_TEST(i, result, ...) \
do {\
const char *myargv[] = { VIRSH_DEFAULT, __VA_ARGS__, NULL };\
const struct testInfo info = { myargv, result };\
@@ -380,10 +386,12 @@ mymain(void)
DO_TEST(30, --shell a\n,
echo \t '-'\-\ \t --shell \t a);
-#undef DO_TEST
+# undef DO_TEST
free(custom_uri);
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
VIRT_TEST_MAIN(mymain)
+
+#endif /* WIN32 */
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virNetClientPtr leak in remote driver
2011/7/27 Matthias Bolte matthias.bo...@googlemail.com:
doRemoteClose doesn't free the virNetClientPtr and this creates a
260kb leak per remote connection. This happens because
virNetClientFree doesn't remove the last ref, because virNetClientNew
creates the virNetClientPtr with a refcount of 2.
static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
const char *hostname)
{
[...]
client-refs = 1;
[...]
/* Set up a callback to listen on the socket data */
client-refs++;
if (virNetSocketAddIOCallback(client-sock,
VIR_EVENT_HANDLE_READABLE,
virNetClientIncomingEvent,
client,
virNetClientEventFree) 0) {
client-refs--;
VIR_DEBUG(Failed to add event watch, disabling events);
}
[...]
}
virNetClientNew adds a ref before calling virNetSocketAddIOCallback
but only removes it when virNetSocketAddIOCallback fails. This seems
wrong too me and the ref should be removed in the success case too.
The same pattern was added in 0302391ee643ad91fdc6d2ecf7e4cf0fc9724516
(Fix race in ref counting when handling RPC jobs)
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -763,10 +763,12 @@ readmore:
/* Send off to for normal dispatch to workers */
if (msg) {
+ client-refs++;
if (!client-dispatchFunc ||
client-dispatchFunc(client, msg,
client-dispatchOpaque) 0) {
virNetMessageFree(msg);
client-wantClose = true;
+ client-refs--;
return;
}
}
Again, this seems wrong and the ref should be removed in the success
case here too. Before I spent time to figure out how the refcounting
is supposed to work here I just report it and hope that Dan can easily
fix this.
Okay, after some discussion on IRC with Michal and Eric and further
testing I think that the ref counting is okay here.
virNetClientNew adds the second ref because the free callback
(virNetClientEventFree) passed to virNetSocketAddIOCallback will call
virNetClientFree.
Another observation is that virNetClientFree calls
virNetSocketRemoveIOCallback when the last ref was removed. Actually
that's pointless because virNetSocketRemoveIOCallback might indirectly
removed the last ref. So this looks like an ordering problem. The
ordering gets fixed by calling virNetClientClose before
virNetClientFree, because virNetClientClose calls
virNetSocketRemoveIOCallback. Another problem is that
virNetSocketRemoveIOCallback only indirectly results in a call to
virNetClientFree, because the event loop will call virNetClientFree
when removing callbacks marked for deletion --
virNetSocketRemoveIOCallback does this marking.
The memory leak I saw was due to virsh calling
virEventRegisterDefaultImpl, but not calling virEventRunDefaultImpl.
Because the event loop is initialized, the call to
virNetSocketAddIOCallback in virNetClientNew succeeds. But virsh not
driving the event loop results in not removing callbacks that were
marked for deletion. Finally this leaks the virNetClientPtr using in
the remote driver. I used a virsh -c qemu:///system to test.
I was able fix this by calling virEventRunDefaultImpl after
virConnectClose in virsh. But I don't think that this is the correct
general solution.
To me the general problem seems to be the entanglement between the
virNetClientPtr refcounting and the event loop. I'm not sure how to
improve this in general. Maybe should have a thread calling
virEventRunDefaultImpl as the comment on virEventRunDefaultImpl
suggest.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Memory leak of the remoteDomainSet* functions
2011/7/27 Osier Yang jy...@redhat.com:
Hello, there
Per bug https://bugzilla.redhat.com/show_bug.cgi?id=725322, should
we change the remoteDomainSet* functions into skipgen, and fix the
leaks like below? (NB, new VIR_FREE statements)
Why not fix the generator instead?
static int
remoteDomainSetBlkioParameters(virDomainPtr dom,
virTypedParameterPtr params,
int nparams,
unsigned int flags)
{
int rv = -1;
int i;
struct private_data *priv = dom-conn-privateData;
remote_domain_set_blkio_parameters_args args;
remote_domain_set_blkio_parameters_args args;
remoteDriverLock(priv);
make_nonnull_domain(args.dom, dom);
args.flags = flags;
if (remoteSerializeTypedParameters(params,
nparams,
args.params.params_val,
args.params.params_len) 0) {
for (i = 0; i nparams; i++) {
VIR_FREE(args.params.params_val[i].field);
}
VIR_FREE(args.params.params_val);
This loop should not be needed as remoteSerializeTypedParameters
cleans up in the error case.
xdr_free((xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char
*)args);
goto done;
}
if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS,
(xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char *)args,
(xdrproc_t)xdr_void, (char *)NULL) == -1) {
goto done;
}
rv = 0;
done:
if (args.params.params_val) {
for (i = 0; i nparams; i++) {
VIR_FREE(args.params.params_val[i].field);
}
VIR_FREE(args.params.params_val);
}
remoteDriverUnlock(priv);
return rv;
}
This can be fixed in the generator, but why no use xdr_free here
instead of open coding the free loop?
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpc: Fix memory leak in remoteDomainSet*Parameters functions
Add a new helper remoteFreeTypedParameters and teach the generator
to add it to the cleanup section.
https://bugzilla.redhat.com/show_bug.cgi?id=725322
---
src/remote/remote_driver.c | 22 +-
src/rpc/gendispatch.pl |5 +
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 0652e0d..e5bfa4b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1208,6 +1208,22 @@ done:
return rv;
}
+/* Helper to free typed parameters. */
+static void
+remoteFreeTypedParameters(remote_typed_param *args_params_val,
+ u_int args_params_len)
+{
+u_int i;
+
+if (args_params_val == NULL)
+return;
+
+for (i = 0; i args_params_len; i++)
+VIR_FREE(args_params_val[i].field);
+
+VIR_FREE(args_params_val);
+}
+
/* Helper to serialize typed parameters. */
static int
remoteSerializeTypedParameters(virTypedParameterPtr params,
@@ -1264,11 +1280,7 @@ remoteSerializeTypedParameters(virTypedParameterPtr
params,
rv = 0;
cleanup:
-if (val) {
-for (i = 0; i nparams; i++)
-VIR_FREE(val[i].field);
-VIR_FREE(val);
-}
+remoteFreeTypedParameters(val, nparams);
return rv;
}
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index ceeb1f5..0d344e8 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -979,6 +979,7 @@ elsif ($opt_k) {
my @args_check_list = ();
my @setters_list = ();
my @setters_list2 = ();
+my @free_list = ();
my $priv_src = conn;
my $priv_name = privateData;
my $call_args = args;
@@ -1105,6 +1106,7 @@ elsif ($opt_k) {
xdr_free((xdrproc_t)xdr_$call-{args}, (char *)args);\n .
goto done;\n .
});
+push(@free_list,
remoteFreeTypedParameters(args.params.params_val, args.params.params_len);\n);
} elsif ($args_member =~ m/^((?:unsigned )?int)
(\S+);\s*\/\*\s*call-by-reference\s*\*\//) {
my $type_name = $1 *;
my $arg_name = $2;
@@ -1500,6 +1502,9 @@ elsif ($opt_k) {
print \n;
print done:\n;
+
+print join(\n, @free_list);
+
print remoteDriverUnlock(priv);\n;
print return rv;\n;
print }\n;
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Memory leak of the remoteDomainSet* functions
2011/7/27 Matthias Bolte matthias.bo...@googlemail.com: 2011/7/27 Osier Yang jy...@redhat.com: Hello, there Per bug https://bugzilla.redhat.com/show_bug.cgi?id=725322, should we change the remoteDomainSet* functions into skipgen, and fix the leaks like below? (NB, new VIR_FREE statements) Why not fix the generator instead? An here's the patch that does this https://www.redhat.com/archives/libvir-list/2011-July/msg01929.html -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: Fix memory leak in remoteDomainSet*Parameters functions
2011/7/27 Eric Blake ebl...@redhat.com: On 07/27/2011 01:18 PM, Matthias Bolte wrote: Add a new helper remoteFreeTypedParameters and teach the generator to add it to the cleanup section. https://bugzilla.redhat.com/show_bug.cgi?id=725322 --- src/remote/remote_driver.c | 22 +- src/rpc/gendispatch.pl | 5 + 2 files changed, 22 insertions(+), 5 deletions(-) ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: expose change-protection during migration
2011/7/27 Eric Blake ebl...@redhat.com: * tools/virsh.c (doMigrate): Add --change-protection flag. * tools/virsh.pod (migrate): Document it. --- As promised here: https://www.redhat.com/archives/libvir-list/2011-July/msg01919.html ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virNetClientPtr leak in remote driver
doRemoteClose doesn't free the virNetClientPtr and this creates a
260kb leak per remote connection. This happens because
virNetClientFree doesn't remove the last ref, because virNetClientNew
creates the virNetClientPtr with a refcount of 2.
static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
const char *hostname)
{
[...]
client-refs = 1;
[...]
/* Set up a callback to listen on the socket data */
client-refs++;
if (virNetSocketAddIOCallback(client-sock,
VIR_EVENT_HANDLE_READABLE,
virNetClientIncomingEvent,
client,
virNetClientEventFree) 0) {
client-refs--;
VIR_DEBUG(Failed to add event watch, disabling events);
}
[...]
}
virNetClientNew adds a ref before calling virNetSocketAddIOCallback
but only removes it when virNetSocketAddIOCallback fails. This seems
wrong too me and the ref should be removed in the success case too.
The same pattern was added in 0302391ee643ad91fdc6d2ecf7e4cf0fc9724516
(Fix race in ref counting when handling RPC jobs)
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -763,10 +763,12 @@ readmore:
/* Send off to for normal dispatch to workers */
if (msg) {
+client-refs++;
if (!client-dispatchFunc ||
client-dispatchFunc(client, msg,
client-dispatchOpaque) 0) {
virNetMessageFree(msg);
client-wantClose = true;
+client-refs--;
return;
}
}
Again, this seems wrong and the ref should be removed in the success
case here too. Before I spent time to figure out how the refcounting
is supposed to work here I just report it and hope that Dan can easily
fix this.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h
2011/6/1 Eric Blake ebl...@redhat.com: On 05/31/2011 02:51 PM, Matthias Bolte wrote: AM_GNU_GETTEXT calls AM_ICONV_LINK. AM_ICONV_LINK saves and alters CPPFLAGS, but doesn't restore it when it finds libiconv. This results in /usr/local/include ending up in the gcc command line before the include path for the local include directory. This makes gcc pick a previous installed libvirt.h instead of the correct one from the source tree. Workaround this problem by saving and restoring CPPFLAGS around the AM_GNU_GETTEXT call. --- configure.ac | 8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index b2ba930..8f46dbd 100644 --- a/configure.ac +++ b/configure.ac @@ -2011,8 +2011,16 @@ dnl Enable building libvirtd? AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd = xyes]) dnl Check for gettext - don't go any newer than what RHEL 5 supports +dnl +dnl save and restore CPPFLAGS around gettext check as the internal iconv +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting +dnl in the build picking up previously installed libvirt/libvirt.h instead +dnl of the correct one from the soucre tree + +save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) +CPPFLAGS=$save_CPPFLAGS Does this still pick up /usr/local/include later in the command line, for the portion of the build that actually needs to include iconf headers, but in a position after our internal headers have been found first? I didn't check, but when we assume that AM_GNU_GETTEXT extends CPPFLAGS for a reason and not just by accident then we need to preserve it's additions to CPPFLAGS but in a later place in the commandline. As the actual problem is that CPPFLAGS comes before -I../include. I was actually thinking that we should instead work around this by setting AM_CPPFLAGS somewhere in our Makefile.am files. That is, when AM_CPPFLAGS is set, then automake outputs $(AM_CPPFLAGS) $(CPPFLAGS) when producing compilation rules, such that our internal CPPFLAGS settings come first in the command line, prior to anything inherited during configure (including the internal iconv settings). That is, with new enough automake, I think this alternative patch solve your BSD compilation (although other directories need the same treatment), without needing any configure.ac hacks. On the other hand, it looks like RHEL 5 only supports automake 1.9.6, and the automake NEWS doesn't mention AM_CPPFLAGS until 1.10 :( So even if the approach in my patch solves your issue, I think it's a non-starter, and something like your patch may be the best we can do. But I'm still worried that we have to use -I/usr/local/include somewhere in the command line, which means we would have to modify src/Makefile.am (and friends) to have: INCLUDES= ... $(GETTEXT_CPPFLAGS) where GETTEXT_CPPFLAGS is substituted with the difference in $save_CPPFLAGS and $CPPFLAGS prior to the point where we restore $CPPFLAGS. That's probably what we should do here. Now how to compute this difference? When we assume that save_CPPFLAGS and CPPFLAGS have a common prefix that we need to strip to get GETTEXT_CPPFLAGS then we could do it like this echo $(CPPFLAGS) | cut -c 1-`expr length $(save_CPPFLAGS)` --complement - This is probably neither the best nor a robustest way to do this. Do you have a better idea? -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: detect gnutls errors
2011/7/25 Daniel P. Berrange berra...@redhat.com: On Fri, Jul 22, 2011 at 11:55:02PM +0200, Matthias Bolte wrote: 2011/7/22 Eric Blake ebl...@redhat.com: * tests/virnettlscontexttest.c (testTLSLoadKey): Report errors. --- Something in gnutls 2.8.5 (RHEL 6) was more leniant than gnutls 2.8.6 (Fedora 14). This still doesn't solve the failure, but at least gets us to see that newer gnutls_x509_privkey_import doesn't like our define of PRIVATE_KEY. Replacing the PRIVATE_KEY with a new one makes the test work better for me with gnutls 2.8.6. I generated the key like this certtool --generate-privkey | sed -e 's/^\(.*\)$/\\1\\n\ \\/' This gives me this output $ ./virnettlscontexttest TEST: virnettlscontexttest !!!. 40 48 FAIL The failing test are those three, that are expected to fail, but don't as it seems /* Expired stuff */ [...] DO_CTX_TEST(true, cacertexpreq, servercertreq, true); DO_CTX_TEST(true, cacertreq, servercertexpreq, true); DO_CTX_TEST(false, cacertreq, clientcertexpreq, true); When we assume that this test worked for Dan with gnutls 2.8.5, what does it means that those three tests are failing for me with gnutls 2.8.6? Here's are some random ideas a) there is a bug in the testcase that causes this b) there is a bug in the tested code in libvirt that causes this c) there is a bug in gnutls 2.8.6 that causes this d) there is a bug in gnutls 2.8.5 that makes the broken test pass e) etc This is a regression introduced by commit 5283ea9b1d8a4b0f2fd6796bf60615aca7b6c3e6 which I have justed fixed in 567b8d69b97827da0e6e7145edb83ec0d7deff86 Okay, this fixes the three failing test cases, but I still need to replace the PRIVATE_KEY, otherwise the test still aborts. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix import of private key with older gnutls
2011/7/25 Daniel P. Berrange berra...@redhat.com: With older GNUTLS the gnutls_x509_privkey_import function is unable to import our private key. Instead we must use the alternative gnutls_x509_privkey_import_pkcs8() (as certtool does). * virnettlscontexttest.c: Fix import of private key with older gnutls. Also add missing newlines to key --- tests/virnettlscontexttest.c | 47 - 1 files changed, 27 insertions(+), 20 deletions(-) ACK, this makes virnettlscontexttest pass for me with gnutls 2.8.6. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Use $(PYTHON) instead of python for the keycode map generator
Also prepend $(AM_V_GEN). --- src/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 481caba..eef0669 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -90,7 +90,7 @@ EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeymaps.h: $(srcdir)/util/keymaps.csv\ $(srcdir)/util/virkeycode-mapgen.py - python $(srcdir)/util/virkeycode-mapgen.py $(srcdir)/util/keymaps.csv $@ + $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py $(srcdir)/util/keymaps.csv $@ $(srcdir)/util/virkeycode.c: $(srcdir)/util/virkeycode.h $(srcdir)/util/virkeymaps.h -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] xenapi: Fix double-freeing the session in xenapiClose
2011/7/21 Eric Blake ebl...@redhat.com:
On 07/21/2011 10:11 AM, Matthias Bolte wrote:
xen_session_logout already frees the whole session object.
Don't call xenSessionFree on a freed session object.
Reported by Sharmila Radhakrishnan.
---
src/xenapi/xenapi_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 97da1d1..dbc03cb 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -230,7 +230,7 @@ xenapiClose (virConnectPtr conn)
if (priv-session != NULL) {
xen_session_logout(priv-session);
- xenSessionFree(priv-session);
+ priv-session = NULL;
}
ACK.
Thanks, pushed.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Use $(PYTHON) instead of python for the keycode map generator
2011/7/22 Eric Blake ebl...@redhat.com: On 07/22/2011 07:56 AM, Matthias Bolte wrote: Also prepend $(AM_V_GEN). --- src/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 481caba..eef0669 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -90,7 +90,7 @@ EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py - python $(srcdir)/util/virkeycode-mapgen.py$(srcdir)/util/keymaps.csv$@ + $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py$(srcdir)/util/keymaps.csv$@ ACK. Hmm, while you're at it, how about you also: chmod +x src/util/virkeycode-mapgen.py to match our style of .py files being executable (even if we run them via $(PYTHON)). Good call. I also switched the shebang from /bin/python to the commonly use /usr/bin/python. I pushed the patch with the mentioned modifications. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Use $(PYTHON) instead of python for the keycode map generator
2011/7/22 Daniel Veillard veill...@redhat.com: On Fri, Jul 22, 2011 at 08:06:00AM -0600, Eric Blake wrote: On 07/22/2011 07:56 AM, Matthias Bolte wrote: Also prepend $(AM_V_GEN). --- src/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 481caba..eef0669 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -90,7 +90,7 @@ EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeymaps.h: $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py - python $(srcdir)/util/virkeycode-mapgen.py$(srcdir)/util/keymaps.csv$@ + $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virkeycode-mapgen.py$(srcdir)/util/keymaps.csv$@ ACK. Ah right, I missed that ! Hmm, while you're at it, how about you also: chmod +x src/util/virkeycode-mapgen.py to match our style of .py files being executable (even if we run them via $(PYTHON)). just wondering loud, if you change a file attribute like this will git automatically detect it and all to commit the change or is there some magic involved (yes I'm think thinking too much in terms of CVS !) Daniel git is smart and knows about the file mode an its changes, see the diff I squashed into the original patch diff --git a/src/util/virkeycode-mapgen.py b/src/util/virkeycode-mapgen.py old mode 100644 new mode 100755 index f0a4290..acf7364 --- a/src/util/virkeycode-mapgen.py +++ b/src/util/virkeycode-mapgen.py @@ -1,4 +1,4 @@ -#!/bin/python +#!/usr/bin/python -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: detect gnutls errors
2011/7/22 Eric Blake ebl...@redhat.com: * tests/virnettlscontexttest.c (testTLSLoadKey): Report errors. --- Something in gnutls 2.8.5 (RHEL 6) was more leniant than gnutls 2.8.6 (Fedora 14). This still doesn't solve the failure, but at least gets us to see that newer gnutls_x509_privkey_import doesn't like our define of PRIVATE_KEY. Replacing the PRIVATE_KEY with a new one makes the test work better for me with gnutls 2.8.6. I generated the key like this certtool --generate-privkey | sed -e 's/^\(.*\)$/\\1\\n\ \\/' This gives me this output $ ./virnettlscontexttest TEST: virnettlscontexttest !!!. 40 48 FAIL The failing test are those three, that are expected to fail, but don't as it seems /* Expired stuff */ [...] DO_CTX_TEST(true, cacertexpreq, servercertreq, true); DO_CTX_TEST(true, cacertreq, servercertexpreq, true); DO_CTX_TEST(false, cacertreq, clientcertexpreq, true); When we assume that this test worked for Dan with gnutls 2.8.5, what does it means that those three tests are failing for me with gnutls 2.8.6? Here's are some random ideas a) there is a bug in the testcase that causes this b) there is a bug in the tested code in libvirt that causes this c) there is a bug in gnutls 2.8.6 that causes this d) there is a bug in gnutls 2.8.5 that makes the broken test pass e) etc I'm not sure. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/7] bandwidth: Declare internal structures
2011/7/18 Jiri Denemark jdene...@redhat.com:
On Tue, Jul 12, 2011 at 13:57:08 +0200, Michal Privoznik wrote:
---
src/util/network.h | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/src/util/network.h b/src/util/network.h
index ed0b78c..568bca1 100644
--- a/src/util/network.h
+++ b/src/util/network.h
@@ -45,6 +45,22 @@ typedef struct {
typedef virSocketAddr *virSocketAddrPtr;
+typedef struct {
+ /* Even if we let user to input rates
+ * in various units, we store them in bps */
Let's not allow users to use various units :-)
ditto
+ unsigned long average;
+ unsigned long peak;
+ unsigned long burst;
+} virRate;
Also don't use long, as it's size is compiler and architecture
dependent. Make it either int if that's large enough or make it long
long to guarantee 64bit size if we need that here.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] python: Fix makefile rule for code generation
Commit 8665f85523f0451c changed generated.stamp to $(GENERATE).stamp, but missed one instance in the CLEANFILES list. This can break the build in case the generated code is deleted but the .stamp file stays around and therefore the code isn't regenerated. --- python/Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/python/Makefile.am b/python/Makefile.am index a4c9a6b..51f005d 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -71,7 +71,7 @@ install-data-local: uninstall-local: rm -f $(DESTDIR)$(pyexecdir)/libvirt.py -CLEANFILES= $(GENERATED) generated.stamp +CLEANFILES= $(GENERATED) $(GENERATE).stamp else all: -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] rpc: Make the dispatch generator handle 'void name(void)' style procedures
The only 'void name(void)' style procedure in the protocol is 'close' that
is handled special, but also programming errors like a missing _args or
_ret suffix on the structs in the .x files can create such a situation by
accident. Making the generator aware of this avoids bogus errors from the
generator such as:
Use of uninitialized value in exists at ./rpc/gendispatch.pl line 967.
Also this allows to get rid of the -c option and the special case code for
the 'close' procedure, as the generator handles it now correctly.
Reported by Michal Privoznik
---
v2:
- remove the special handling of the 'close' procedure
daemon/Makefile.am |2 +-
src/Makefile.am|2 +-
src/rpc/gendispatch.pl | 32 +---
3 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 8ed29b8..63c731e 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -44,7 +44,7 @@ QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x
$(srcdir)/remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \
$(REMOTE_PROTOCOL)
- $(AM_V_GEN)perl -w $(srcdir)/../src/rpc/gendispatch.pl -c -b remote \
+ $(AM_V_GEN)perl -w $(srcdir)/../src/rpc/gendispatch.pl -b remote \
$(REMOTE_PROTOCOL) $@
$(srcdir)/qemu_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \
diff --git a/src/Makefile.am b/src/Makefile.am
index fadde28..00e78ac 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -188,7 +188,7 @@ REMOTE_DRIVER_PROTOCOL = $(REMOTE_PROTOCOL) $(QEMU_PROTOCOL)
$(srcdir)/remote/remote_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \
$(REMOTE_PROTOCOL)
$(AM_V_GEN)perl -w $(srcdir)/rpc/gendispatch.pl \
- -c -k remote $(REMOTE_PROTOCOL) $@
+ -k remote $(REMOTE_PROTOCOL) $@
$(srcdir)/remote/qemu_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \
$(QEMU_PROTOCOL)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index e068b53..6e26e2d 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -9,7 +9,7 @@
# for both remote_protocol.x and qemu_protocol.x, you would run the
# following:
#
-# gendispatch.pl -c -t remote ../src/remote/remote_protocol.x
+# gendispatch.pl -t remote ../src/remote/remote_protocol.x
# gendispatch.pl -t qemu ../src/remote/qemu_protocol.x
#
# By Richard Jones rjo...@redhat.com
@@ -20,8 +20,8 @@ use strict;
use Getopt::Std;
# Command line options.
-our ($opt_p, $opt_t, $opt_a, $opt_r, $opt_d, $opt_c, $opt_b, $opt_k);
-getopts ('ptardcbk');
+our ($opt_p, $opt_t, $opt_a, $opt_r, $opt_d, $opt_b, $opt_k);
+getopts ('ptardbk');
my $structprefix = shift or die missing prefix argument;
my $protocol = shift or die missing protocol argument;
@@ -45,18 +45,6 @@ sub name_to_ProcName {
# opinion about the name, args and return type of each RPC.
my ($name, $ProcName, $id, $flags, %calls, @calls);
-# only generate a close method if -c was passed
-if ($opt_c) {
-# REMOTE_PROC_CLOSE has no args or ret.
-$calls{close} = {
-name = close,
-ProcName = Close,
-UC_NAME = CLOSE,
-args = void,
-ret = void,
-};
-}
-
my $collect_args_members = 0;
my $collect_ret_members = 0;
my $last_name;
@@ -143,6 +131,20 @@ while (PROTOCOL) {
$flags = $3;
$ProcName = name_to_ProcName ($name);
+if (!exists $calls{$name}) {
+# that the argument and return value cases have not yet added
+# this procedure to the calls hash means that it has no arguments
+# and no return value. add it to the calls hash now because all
+# procedures have to be listed in the calls hash
+$calls{$name} = {
+name = $name,
+ProcName = $ProcName,
+UC_NAME = uc $name,
+args = void,
+ret = void
+}
+}
+
if ($opt_b or $opt_k) {
if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(.*)\*\/\s*$/)) {
die invalid generator flags for ${procprefix}_PROC_${name}
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] python: Fix makefile rule for code generation
2011/7/21 Daniel P. Berrange berra...@redhat.com: On Thu, Jul 21, 2011 at 01:47:14PM +0200, Matthias Bolte wrote: Commit 8665f85523f0451c changed generated.stamp to $(GENERATE).stamp, but missed one instance in the CLEANFILES list. This can break the build in case the generated code is deleted but the .stamp file stays around and therefore the code isn't regenerated. --- python/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/python/Makefile.am b/python/Makefile.am index a4c9a6b..51f005d 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -71,7 +71,7 @@ install-data-local: uninstall-local: rm -f $(DESTDIR)$(pyexecdir)/libvirt.py -CLEANFILES= $(GENERATED) generated.stamp +CLEANFILES= $(GENERATED) $(GENERATE).stamp else all: ACK Daniel Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] xenapi: Improve error reporting in xenapiOpen once again
privP-session-error_description is a list and in order to get the
complete error message all parts of the list should be concatenated.
xenapiSessionErrorHandler does this when its third parameter is NULL.
The current code discards all but the first part of the error message
resulting in a potentially incomplete error message.
This partly reverts 006be75ee214f9b4, that tried to avoid reporting
a (null) in the error message. The actual problem is more general in
returnErrorFromSession that might return NULL if there is no error.
Make sure that returnErrorFromSession return non-NULL always. Also
don't skip the last error message part.
---
src/xenapi/xenapi_driver.c |5 +
src/xenapi/xenapi_utils.c |4 +++-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 97da1d1..77bf82d 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -193,10 +193,7 @@ xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth,
return VIR_DRV_OPEN_SUCCESS;
}
-xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED,
- *privP-session-error_description != NULL ?
- *privP-session-error_description :
- _(unknown error));
+xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED, NULL);
error:
VIR_FREE(username);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index 342ae5b..79fd946 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -272,12 +272,14 @@ returnErrorFromSession(xen_session *session)
{
int i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-for (i = 0; i session-error_description_count - 1; i++) {
+for (i = 0; i session-error_description_count; i++) {
if (!i)
virBufferEscapeString(buf, %s, session-error_description[i]);
else
virBufferEscapeString(buf, : %s,
session-error_description[i]);
}
+if (virBufferUse(buf) 1)
+virBufferAdd(buf, _(unknown error), -1);
return virBufferContentAndReset(buf);
}
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] rpc: Make the dispatch generator handle 'void name(void)' style procedures
2011/7/21 Eric Blake ebl...@redhat.com: On 07/21/2011 05:58 AM, Matthias Bolte wrote: The only 'void name(void)' style procedure in the protocol is 'close' that is handled special, but also programming errors like a missing _args or _ret suffix on the structs in the .x files can create such a situation by accident. Making the generator aware of this avoids bogus errors from the generator such as: Use of uninitialized value in exists at ./rpc/gendispatch.pl line 967. Also this allows to get rid of the -c option and the special case code for the 'close' procedure, as the generator handles it now correctly. Reported by Michal Privoznik --- v2: - remove the special handling of the 'close' procedure daemon/Makefile.am | 2 +- src/Makefile.am | 2 +- src/rpc/gendispatch.pl | 32 +--- 3 files changed, 19 insertions(+), 17 deletions(-) ACK. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] xenapi: Fix double-freeing the session in xenapiClose
xen_session_logout already frees the whole session object.
Don't call xenSessionFree on a freed session object.
Reported by Sharmila Radhakrishnan.
---
src/xenapi/xenapi_driver.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 97da1d1..dbc03cb 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -230,7 +230,7 @@ xenapiClose (virConnectPtr conn)
if (priv-session != NULL) {
xen_session_logout(priv-session);
-xenSessionFree(priv-session);
+priv-session = NULL;
}
VIR_FREE(priv-url);
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: avoid fds leak on virEventPollInit
2011/7/19 a...@redhat.com:
* src/util/event_poll.c: fix file descriptors leak on virEventPollInit.
Detected in valgrind run:
==1254==
==1254== FILE DESCRIPTORS: 6 open at exit.
==1254== Open file descriptor 5:
==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82)
==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61)
==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648)
==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208)
==1254== by 0x42150C: main (virsh.c:13790)
==1254==
==1254== Open file descriptor 4:
==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82)
==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61)
==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648)
==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208)
==1254== by 0x42150C: main (virsh.c:13790)
==1254==
* how to reproduce?
% valgrind -v --track-fds=yes virsh list
---
src/util/event_poll.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 285ba50..1e4ef96 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -639,6 +639,8 @@ static void virEventPollHandleWakeup(int watch
ATTRIBUTE_UNUSED,
int virEventPollInit(void)
{
+ int ret = -1;
+
if (virMutexInit(eventLoop.lock) 0) {
virReportSystemError(errno, %s,
_(Unable to initialize mutex));
@@ -648,7 +650,7 @@ int virEventPollInit(void)
if (pipe2(eventLoop.wakeupfd, O_CLOEXEC | O_NONBLOCK) 0) {
virReportSystemError(errno, %s,
_(Unable to setup wakeup pipe));
- return -1;
+ goto cleanup;
}
if (virEventPollAddHandle(eventLoop.wakeupfd[0],
@@ -657,10 +659,15 @@ int virEventPollInit(void)
virEventError(VIR_ERR_INTERNAL_ERROR,
_(Unable to add handle %d to event loop),
eventLoop.wakeupfd[0]);
- return -1;
+ goto cleanup;
}
- return 0;
+ ret = 0;
+
+cleanup:
+ close(eventLoop.wakeupfd[0]);
+ close(eventLoop.wakeupfd[1]);
+ return ret;
}
NACK, as this is wrong. You're closing the pipe in all cases in
virEventPollInit, but the pipe is supposed to be used afterwards. As
there is no virEventPollDeinit this FD leak has to be considered as a
static leak that's expected and not to be fixed. Also close() is
forbidden and make syntax-check should have told you that.
What can be done here is closing the pipe in case
virEventPollAddHandle fails, for example like this:
diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 285ba50..a62f960 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -657,6 +657,8 @@ int virEventPollInit(void)
virEventError(VIR_ERR_INTERNAL_ERROR,
_(Unable to add handle %d to event loop),
eventLoop.wakeupfd[0]);
+VIR_CLOSE(eventLoop.wakeupfd[0]);
+VIR_CLOSE(eventLoop.wakeupfd[1]);
return -1;
}
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: avoid fds leak on virEventPollInit
2011/7/19 Daniel Veillard veill...@redhat.com:
On Tue, Jul 19, 2011 at 11:00:21AM +0200, Matthias Bolte wrote:
2011/7/19 a...@redhat.com:
* src/util/event_poll.c: fix file descriptors leak on virEventPollInit.
Detected in valgrind run:
==1254==
==1254== FILE DESCRIPTORS: 6 open at exit.
==1254== Open file descriptor 5:
==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82)
==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61)
==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648)
==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208)
==1254== by 0x42150C: main (virsh.c:13790)
==1254==
==1254== Open file descriptor 4:
==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82)
==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61)
==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648)
==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208)
==1254== by 0x42150C: main (virsh.c:13790)
==1254==
* how to reproduce?
% valgrind -v --track-fds=yes virsh list
---
src/util/event_poll.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 285ba50..1e4ef96 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -639,6 +639,8 @@ static void virEventPollHandleWakeup(int watch
ATTRIBUTE_UNUSED,
int virEventPollInit(void)
{
+ int ret = -1;
+
if (virMutexInit(eventLoop.lock) 0) {
virReportSystemError(errno, %s,
_(Unable to initialize mutex));
@@ -648,7 +650,7 @@ int virEventPollInit(void)
if (pipe2(eventLoop.wakeupfd, O_CLOEXEC | O_NONBLOCK) 0) {
virReportSystemError(errno, %s,
_(Unable to setup wakeup pipe));
- return -1;
+ goto cleanup;
}
if (virEventPollAddHandle(eventLoop.wakeupfd[0],
@@ -657,10 +659,15 @@ int virEventPollInit(void)
virEventError(VIR_ERR_INTERNAL_ERROR,
_(Unable to add handle %d to event loop),
eventLoop.wakeupfd[0]);
- return -1;
+ goto cleanup;
}
- return 0;
+ ret = 0;
+
+cleanup:
+ close(eventLoop.wakeupfd[0]);
+ close(eventLoop.wakeupfd[1]);
+ return ret;
}
NACK, as this is wrong. You're closing the pipe in all cases in
virEventPollInit, but the pipe is supposed to be used afterwards. As
there is no virEventPollDeinit this FD leak has to be considered as a
static leak that's expected and not to be fixed. Also close() is
forbidden and make syntax-check should have told you that.
What can be done here is closing the pipe in case
virEventPollAddHandle fails, for example like this:
diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 285ba50..a62f960 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -657,6 +657,8 @@ int virEventPollInit(void)
virEventError(VIR_ERR_INTERNAL_ERROR,
_(Unable to add handle %d to event loop),
eventLoop.wakeupfd[0]);
+ VIR_CLOSE(eventLoop.wakeupfd[0]);
+ VIR_CLOSE(eventLoop.wakeupfd[1]);
return -1;
}
The only problem I have is why is a simple virsh list exhibiting
one of those 2 errors, they sounds like hard cornercases, and not
something to be triggered in such a simple operation. Valgrind had to
emulate one of those code path to raise the error, and that sounds
weird.
Daniel
I'm not sure that I understand what you mean. The FD leak that
valgrind detected is there and expected as virsh (indirectly) calls
virEventPollInit, but there is not counterpart to virEventPollInit to
close those FDs again. This is an expected static leak, you're
supposed to call virEventPollInit at most once in your program. So
there is actually nothing to fix here.
The patch I suggested is unrelated to the valfrind detected leak. In
case virEventPollAddHandle fails we know that virEventPollInit will
fail and that the calling app should fail too. In that case we can
close the FDs. This is just an 'improvement' that's not strictly
necessary. It just avoids the static leak in an error case.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpc: Make the dispatch generator handle 'void name(void)' style procedures
Currently there are no such procedures in the protocol, but programming
errors like a missing _args or _ret suffix on the structs in the .x files
can create such a situation by accident. Making the generator aware of
this avoids bogus errors from the generator such as:
Use of uninitialized value in exists at ./rpc/gendispatch.pl line 967.
Reported by Michal Privoznik
---
src/rpc/gendispatch.pl | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index e068b53..e6a4a1c 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -143,6 +143,20 @@ while (PROTOCOL) {
$flags = $3;
$ProcName = name_to_ProcName ($name);
+if (!exists $calls{$name}) {
+# that the argument and return value cases have not yet added
+# this procedure to the calls hash means that it has no arguments
+# and no return value. add it to the calls hash now because all
+# procedures have to be listed in the calls hash
+$calls{$name} = {
+name = $name,
+ProcName = $ProcName,
+UC_NAME = uc $name,
+args = void,
+ret = void
+}
+}
+
if ($opt_b or $opt_k) {
if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(.*)\*\/\s*$/)) {
die invalid generator flags for ${procprefix}_PROC_${name}
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Quieten build ensure API build scripts exit with non-zero status
2011/7/19 Daniel P. Berrange berra...@redhat.com: From: Daniel P. Berrange berra...@redhat.com The current API build scripts will continue and exit with a zero status even if they find problems. This has been the cause of many build problems, or hidden build errors, in the past. Change the scripts so they always exit with a non-zero status for any problems they do not understand. Also turn off all debug output by default so they respect $(AM_V_GEN) * docs/Makefile.am: Use $(AM_V_GEN) for API/HTML scripts * docs/apibuild.py, python/generator.py: Exit with non-zero status if problems are found. Also be silent, not outputting any debug messages. * src/Makefile.am: Use $(AM_V_GEN) for ESX generator * python/Makefile.am: Tweak rule --- docs/Makefile.am | 11 ++ docs/apibuild.py | 95 ++ python/Makefile.am | 6 ++-- python/generator.py | 16 +--- src/Makefile.am | 2 +- 5 files changed, 75 insertions(+), 55 deletions(-) My first review still applies https://www.redhat.com/archives/libvir-list/2011-July/msg00139.html which said ACK with comments on some stylistic things. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: avoid memory leak on cleanup
2011/7/15 a...@redhat.com:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from
networkGetBridgeName function.
---
src/network/bridge_driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0a12bc0..59e780d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) {
cleanup:
if (network)
virNetworkObjUnlock(network);
- return bridge;
+ VIR_FREE(bridge);
+ return NULL;
}
static int networkGetAutostart(virNetworkPtr net,
NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: avoid memory leak on cleanup
2011/7/15 Alex Jia a...@redhat.com:
On 07/15/2011 03:03 PM, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15 a...@redhat.com:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from
networkGetBridgeName function.
---
src/network/bridge_driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0a12bc0..59e780d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) {
cleanup:
if (network)
virNetworkObjUnlock(network);
- return bridge;
+ VIR_FREE(bridge);
+ return NULL;
}
static int networkGetAutostart(virNetworkPtr net,
NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run:
==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26
==9020==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==9020==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==9020==by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30)
==9020==by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret
(remote_protocol.c:1999)
==9020==by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286)
==9020==by 0x4D03235: virNetClientProgramCall
(virnetclientprogram.c:318)
==9020==by 0x4CE7262: call (remote_driver.c:3925)
==9020==by 0x4CED8D2: remoteNetworkGetBridgeName
(remote_client_bodies.h:3384)
==9020==by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503)
==9020==by 0x40F654: cmdNetworkInfo (virsh.c:5015)
==9020==by 0x410D02: vshCommandRun (virsh.c:12738)
==9020==by 0x41F2D5: main (virsh.c:14084)
The caller of virNetworkGetBridgeName has to free the allocated string:
diff --git a/tools/virsh.c b/tools/virsh.c
index bd6fea7..5c48f0c 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -5011,6 +5011,7 @@ cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd)
if (bridge)
vshPrint(ctl, %-15s %s\n, _(Bridge:), bridge);
+VIR_FREE(bridge);
virNetworkFree(network);
return true;
}
==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26
==9020==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==9020==by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so)
==9020==by 0x4CF508B: doRemoteOpen (remote_driver.c:364)
==9020==by 0x4CF6E8F: remoteOpen (remote_driver.c:800)
==9020==by 0x4CCB384: do_open (libvirt.c:1054)
==9020==by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280)
==9020==by 0x410BC0: vshReconnect (virsh.c:589)
==9020==by 0x410DCF: vshCommandRun (virsh.c:12733)
==9020==by 0x41F2D5: main (virsh.c:14084)
==9020==
==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost
in loss record 17 of 26
==9020==at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==9020==by 0x4C686ED: virAlloc (memory.c:101)
==9020==by 0x4C96870: virDomainEventStateNew (domain_event.c:578)
==9020==by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658)
==9020==by 0x4CF6E8F: remoteOpen (remote_driver.c:800)
==9020==by 0x4CCB384: do_open (libvirt.c:1054)
==9020==by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280)
==9020==by 0x410BC0: vshReconnect (virsh.c:589)
==9020==by 0x410DCF: vshCommandRun (virsh.c:12733)
==9020==by 0x41F2D5: main (virsh.c:14084)
Note: remote memory leak are a known bug:
https://bugzilla.redhat.com/show_bug.cgi?id=690734,
so this patch only fix network memory leak.
Judging by a quick look at the code this two leaks should not be
possible? Are you working on current git? Anyway, I'll leave those to
you.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Implement virDomainGetIPAddress()
2011/7/15 Michal Novotny minov...@redhat.com:
On 07/14/2011 05:42 PM, Matthias Bolte wrote:
2011/7/14 Michal Novotny minov...@redhat.com:
Hi guys,
some time ago I've been investigating the options to get the guest's IP
address information without having to connect to guest's VNC window or
console. It was for one project I've been working on and I found that
the solution lies in the procfs, precisely in the /proc/{PID}/net/arp...
The format is as follows:
$ cat /proc/{PID}/net/arp
IP address HW type Flags HW address Mask
Device
192.168.122.36 0x1 0x2 52:54:00:35:76:e6 *
virbr0
where the HW address matches the MAC address associated to the guest's
NIC. Implementing such an API shouldn't be a big problem however I know
that there's some option to run libvirt on Windows machines. It should
be just the client so it shouldn't really matter however I'd like to ask
you whether it's really not an issue.
Windows or not is irrelevant here as the IP address lookup cannot be
implemented in a general way/place, but will have to be implemented by
the hypervisor/network drivers.
The function should return a string of the guest's IP address as read
from the procfs or return a NULL value if there's no IP address
associated with the guest.
If the multiple NICs are being used by the guest then the function
should return either the IP address matching the MAC address passed to
the function or the first IP address if omitted.
The prototype should be:
char *virDomainGetIPAddress(virDomainPtr domain, char *devmac);
First of all you're missing the unsigned int flags parameter.
Also did you consider that the MAC to IP(v4|v6) mapping isn't
necessarily a 1:1 mapping, but the signature of your function requires
this?
Well, for this I considered the IPv4 address only.
I took a look at this and wonder what's the difference between
/proc/{PID}/net/arp and /proc/net/arp. Also as it's ARP it'll only
work for IPv4.
Honestly I was unable to see it in /proc/net/arp. I saw just some ARP
records but not related to the qemu-kvm process I tried on my Fedora-14
box but I was able to see those records in /proc/{PID}/net/arp and
therefore I was looking to the /proc/{PID}/net/arp and why I mentioned
this instead.
Considering the fact this will be working just for IPv4 there should be
some prototype like:
char *virDomainGetIPAddress(virDomainPtr domain, char *devmac, unsigned int
flags);
Even if you're going to restrict it to IPv4 for now this signature
won't do, as you can have multiple IPv4 addresses assigned to the same
MAC address. How do you want to return multiple IP addresses? As a
comma separated list as string and the caller has to parse it?
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Implement virDomainGetIPAddress()
2011/7/15 Michal Novotny minov...@redhat.com:
On 07/15/2011 10:28 AM, Matthias Bolte wrote:
2011/7/15 Michal Novotny minov...@redhat.com:
On 07/14/2011 05:42 PM, Matthias Bolte wrote:
2011/7/14 Michal Novotny minov...@redhat.com:
Hi guys,
some time ago I've been investigating the options to get the guest's IP
address information without having to connect to guest's VNC window or
console. It was for one project I've been working on and I found that
the solution lies in the procfs, precisely in the /proc/{PID}/net/arp...
The format is as follows:
$ cat /proc/{PID}/net/arp
IP address HW type Flags HW address Mask
Device
192.168.122.36 0x1 0x2 52:54:00:35:76:e6 *
virbr0
where the HW address matches the MAC address associated to the guest's
NIC. Implementing such an API shouldn't be a big problem however I know
that there's some option to run libvirt on Windows machines. It should
be just the client so it shouldn't really matter however I'd like to ask
you whether it's really not an issue.
Windows or not is irrelevant here as the IP address lookup cannot be
implemented in a general way/place, but will have to be implemented by
the hypervisor/network drivers.
The function should return a string of the guest's IP address as read
from the procfs or return a NULL value if there's no IP address
associated with the guest.
If the multiple NICs are being used by the guest then the function
should return either the IP address matching the MAC address passed to
the function or the first IP address if omitted.
The prototype should be:
char *virDomainGetIPAddress(virDomainPtr domain, char *devmac);
First of all you're missing the unsigned int flags parameter.
Also did you consider that the MAC to IP(v4|v6) mapping isn't
necessarily a 1:1 mapping, but the signature of your function requires
this?
Well, for this I considered the IPv4 address only.
I took a look at this and wonder what's the difference between
/proc/{PID}/net/arp and /proc/net/arp. Also as it's ARP it'll only
work for IPv4.
Honestly I was unable to see it in /proc/net/arp. I saw just some ARP
records but not related to the qemu-kvm process I tried on my Fedora-14
box but I was able to see those records in /proc/{PID}/net/arp and
therefore I was looking to the /proc/{PID}/net/arp and why I mentioned
this instead.
Considering the fact this will be working just for IPv4 there should be
some prototype like:
char *virDomainGetIPAddress(virDomainPtr domain, char *devmac, unsigned int
flags);
Even if you're going to restrict it to IPv4 for now this signature
won't do, as you can have multiple IPv4 addresses assigned to the same
MAC address. How do you want to return multiple IP addresses? As a
comma separated list as string and the caller has to parse it?
Wait a minute there please. The MAC address should be unique in the
system so user should be using just one IP address per one MAC address.
Since MAC address should be unique in the system then the IP address
assigned to this MAC address should be just one, shouldn't it ?
You can easily add multiple IP address to an interface (and therefore
to the same MAC address)
ip addr add 192.168.0.17 dev eth0
ip addr add 192.168.0.42 dev eth0
Nevertheless I think you know more about networking options than I do so
Well, I know that there is no strict 1:1 mapping between MAC and IP
addresses and I want that this fact is consider in the discussion here
and we don't add a new API that turns out to be too simple/restricted
in the end. Whether we really want/need to cover this case is a
different question.
when I consider the scenario you wrote me about I don't like the idea of
leaving the parsing to the caller and for the scenario of multiple IP
addresses in the return value I recommend a new prototype:
char **virDomainGetIPAddress(virDomainPtr domain, char *devmac, unsigned int
*count, unsigned int flags);
where count will be the output parameter with the number of elements in
the return value. It should be used like:
virDomainPtr domain = ...;
char *macaddr = 11:22:33:44:55:66;
char **ips = NULL;
int count = -1;
ips = virDomainGetIPAddress(domain, macaddr, count, 0);
for (i = 0; i count; i++) {
... ips[i] ...
... free(ips[i]) ...
}
The return value allocation should be done by the function itself and
the called should free the result (ips in this case).
That's a possible way to deal with this, yes.
And now lets spin this a bit further and consider IPv6 addresses.
ipv4s = virDomainGetIPAddress(domain, macaddr, count, VIR_DOMAIN_ADDRESS_IPV4);
ipv6s = virDomainGetIPAddress(domain, macaddr, count, VIR_DOMAIN_ADDRESS_IPV6);
This are the simple cases where the caller explicitly requests only
one version and knows the version of the returned IP addresses. But
what about this
ips = virDomainGetIPAddress(domain, macaddr, count,
VIR_DOMAIN_ADDRESS_IPV4 | VIR_DOMAIN_ADDRESS_IPV6
Re: [libvirt] [PATCH] util: fix a typo error on virStrncpy
2011/7/15 a...@redhat.com: * src/util/util.c: fix a typo error on virStrncpy. --- src/util/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 0ca81df..1080823 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1754,7 +1754,7 @@ virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) return NULL; ret = strncpy(dest, src, n); - /* strncpy NULL terminates iff the last character is \0. Therefore + /* strncpy NULL terminates if the last character is \0. Therefore This is not necessarily a typo. iff is short for 'if and only if'. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]: Add another portion virStream bindings (C# bindings, libvirt-csharp.git)
2011/7/14 Anthony Fox a.podava...@gmail.com: ACK and applied, thanks. The comment to virStreamRecv misses two lines in the returns block, so I'm squashing this in (this diff will probably come through mangled because of the long lines) diff --git a/Stream.cs b/Stream.cs index b349605..d824726 100644 --- a/Stream.cs +++ b/Stream.cs @@ -109,6 +109,8 @@ namespace Libvirt /// size of @data buffer /// /param /// returns + /// the number of bytes read, which may be less than requested.Returns 0 when the end of the stream + /// is reached, at which time the caller should invoke virStreamFinish() to get confirmation of stream /// completion. Returns -1 upon error, at which time the stream will be marked as aborted, /// and the caller should now release the stream with virStreamFree. Returns -2 if there is no /// data pending to be read the stream is marked as non-blocking. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] interface: Check for interface (in-)activity on some operations
2011/7/15 Eric Blake ebl...@redhat.com: On 07/15/2011 08:45 AM, Eric Blake wrote: On 07/15/2011 08:36 AM, Michal Privoznik wrote: On the other hand, if we don't support transient interfaces, then the above analysis which works for domains would have to be adjusted for interfaces, so you may have something to patch after all. Well, although we have function interfaceCreate, it is actually (from semantic POV) interfaceStart, because it just start inactive but defined interface. So we do not support transient interfaces. Therefore transitions for interfaces are slightly different from transitions for domains. That's why I think we do need this patch. Let's nail down the transitions that we plan to support, then, just as I did earlier for domains. It would be even cooler to have a life cycle diagram with the API used to transition between states documented somewhere. I seem to recall seeing one for domains once, but couldn't find it in 5 minutes of searching right now. Found it: http://libvirt.org/guide/html-single/#Application_Development_Guide-Guest_Domains-Lifecycle For some reason the anchor doesn't work for me, just for reference a link that involves less scrolling :) http://libvirt.org/guide/html/Application_Development_Guide-Guest_Domains-Lifecycle.html That diagram completely lacks transient domains. It also shows persistent/running to persistent/inactive via Shutdown, while I mentioned Destroy (both APIs work for that transition, although Shutdown requires guest response). Something that could certainly use some TLC! Also that diagram is wrong about the saved state. There is nothing like that in context of virDomainSave/virDomainRestore. Also the diagram misses managedsave. In the context of managedsave there is actually a saved state. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] interface: Check for interface (in-)activity on some operations
2011/7/15 Eric Blake ebl...@redhat.com: On 07/15/2011 08:48 AM, Michal Privoznik wrote: On 15.07.2011 16:45, Eric Blake wrote: On 07/15/2011 08:36 AM, Michal Privoznik wrote: On the other hand, if we don't support transient interfaces, then the above analysis which works for domains would have to be adjusted for interfaces, so you may have something to patch after all. Well, although we have function interfaceCreate, it is actually (from semantic POV) interfaceStart, because it just start inactive but defined interface. So we do not support transient interfaces. Therefore transitions for interfaces are slightly different from transitions for domains. That's why I think we do need this patch. Let's nail down the transitions that we plan to support, then, just as I did earlier for domains. It would be even cooler to have a life cycle diagram with the API used to transition between states documented somewhere. I seem to recall seeing one for domains once, but couldn't find it in 5 minutes of searching right now. You mean http://wiki.libvirt.org/page/VM_lifecycle#States_that_a_guest_domain_can_be_in You found a different one than me, but that one also has the problem of no mention of a transient domain. That one actually covers transient domain partly. Look at the create/shutdown transition between undefined and running states. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/6] flags: use common dumpxml flags check
2011/7/15 Eric Blake ebl...@redhat.com: The previous patches only cleaned up ATTRIBUTE_UNUSED flags cases; auditing the drivers found other places where flags was being used but not validated. In particular, domainGetXMLDesc had issues with clients accepting a different set of flags than the common virDomainDefFormat helper function. * src/conf/domain_conf.c (virDomainDefFormat): Add common flag check. * src/storage/storage_driver.c (storageVolumeCreateXMLFrom): Pass 0 to drivers, since all flags are currently rejected. * src/uml/uml_driver.c (umlDomainAttachDeviceFlags) (umlDomainDetachDeviceFlags): Reject unknown flags. * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc) (vboxDomainAttachDeviceFlags) (vboxDomainDetachDeviceFlags): Likewise. * src/qemu/qemu_driver.c (qemudDomainMemoryPeek): Likewise. (qemuDomainGetXMLDesc): Document common flag handling. * src/libxl/libxl_driver.c (libxlDomainGetXMLDesc): Likewise. * src/lxc/lxc_driver.c (lxcDomainGetXMLDesc): Likewise. * src/openvz/openvz_driver.c (openvzDomainGetXMLDesc): Likewise. * src/phyp/phyp_driver.c (phypDomainGetXMLDesc): Likewise. * src/test/test_driver.c (testDomainGetXMLDesc): Likewise. * src/vmware/vmware_driver.c (vmwareDomainGetXMLDesc): Likewise. * src/xenapi/xenapi_driver.c (xenapiDomainGetXMLDesc): Likewise. --- diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4f35be0..419df42 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1522,7 +1522,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, virStoragePoolObjUnlock(origpool); } - buildret = backend-buildVolFrom(obj-conn, pool, newvol, origvol, flags); + buildret = backend-buildVolFrom(obj-conn, pool, newvol, origvol, 0); storageDriverLock(driver); virStoragePoolObjLock(pool); I don't think that this is a good idea. Even if the function doesn't have any flags at the moment this change will give us trouble when someone want's to add a flag to virStorageVolCreateXMLFrom that should have been passed down to the backend's buildVolFrom function. ACK, to a patch without this hunk. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 2/6] flags: simplify qemu migration flags
2011/7/15 Eric Blake ebl...@redhat.com: * src/qemu/qemu_driver.c (QEMU_MIGRATION_FLAGS): New define. Simplify all migration callbacks. --- v3: new patch src/qemu/qemu_driver.c | 101 ++-- 1 files changed, 21 insertions(+), 80 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 3/6] esx: reject unknown flags
2011/7/15 Eric Blake ebl...@redhat.com: Silently ignored flags get in the way of new features that use those flags. Regarding ESX migration flags - right now, ESX silently enforces VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE, and VIR_MIGRATE_LIVE, even if those flags were not supplied; it ignored other flags. This patch does not change the implied bits (it permits but does not require them), but enforces only the supported bits. If further cleanup is needed to be more particular about migration flags, that should be a separate patch. * src/esx/esx_device_monitor.c (esxDeviceOpen): Reject unknown flags. * src/esx/esx_driver.c (esxOpen, esxDomainReboot) (esxDomainXMLFromNative, esxDomainXMLToNative) (esxDomainMigratePrepare, esxDomainMigratePerform) (esxDomainMigrateFinish): Likewise. * src/esx/esx_interface_driver.c (esxInterfaceOpen): Likewise. * src/esx/esx_network_driver.c (esxNetworkOpen): Likewise. * src/esx/esx_nwfilter_driver.c (esxNWFilterOpen): Likewise. * src/esx/esx_secret_driver.c (esxSecretOpen): Likewise. * src/esx/esx_storage_driver.c (esxStorageOpen): Likewise. --- v3: address concerns in v2 about migration flags src/esx/esx_device_monitor.c | 4 +++- src/esx/esx_driver.c | 35 --- src/esx/esx_interface_driver.c | 4 +++- src/esx/esx_network_driver.c | 4 +++- src/esx/esx_nwfilter_driver.c | 4 +++- src/esx/esx_secret_driver.c | 4 +++- src/esx/esx_storage_driver.c | 4 +++- 7 files changed, 46 insertions(+), 13 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 4/6] xen: reject unknown flags
2011/7/15 Eric Blake ebl...@redhat.com:
Also fix a logic bug in xenXMDomain{Attach,Detach}DeviceFlags,
where (flags VIR_DOMAIN_DEVICE_MODIFY_CURRENT) is always false.
* src/xen/xen_driver.c (xenUnifiedDomainXMLFromNative)
(xenUnifiedDomainXMLToNative, xenUnifiedDomainBlockPeek): Reject
unknown flags.
* src/xen/xen_hypervisor.c (xenHypervisorOpen)
(xenHypervisorGetDomainState): Likewise.
* src/xen/xen_inotify.c (xenInotifyOpen): Likewise.
* src/xen/xs_internal.c (xenStoreOpen, xenStoreDomainGetState)
(xenStoreDomainReboot): Likewise.
* src/xen/xend_internal.c (xenDaemonOpen, xenDaemonDomainReboot)
(xenDaemonDomainCoreDump, xenDaemonDomainGetState)
(xenDaemonDomainMigratePrepare, xenDaemonDomainSetVcpusFlags,
xenDaemonDomainGetVcpusFlags, xenDaemonAttachDeviceFlags,
xenDaemonDetachDeviceFlags): Likewise.
(xenDaemonDomainGetXMLDesc): Prefer unsigned flags.
* src/xen/xend_internal.h (xenDaemonDomainGetXMLDesc): Likewise.
* src/xen/xm_internal.h (xenXMDomainGetXMLDesc): Likewise.
* src/xen/xm_internal.c (xenXMDomainGetXMLDesc): Likewise.
(xenXMOpen, xenXMDomainGetState, xenXMDomainSetVcpusFlags)
(xenXMDomainGetVcpusFlags): Reject unknown flags.
(xenXMDomainAttachDeviceFlags, xenXMDomainDetachDeviceFlags):
Likewise, and avoid always-false conditional.
* src/xen/xen_driver.h (XEN_MIGRATION_FLAGS): New define.
---
v3: address concerns about migration and coredump flags
src/xen/xen_driver.c | 18 +++---
src/xen/xen_driver.h | 7 +++
src/xen/xen_hypervisor.c | 8 ++--
src/xen/xen_inotify.c | 4 +++-
src/xen/xend_internal.c | 42 ++
src/xen/xend_internal.h | 3 ++-
src/xen/xm_internal.c | 33 ++---
src/xen/xm_internal.h | 2 +-
src/xen/xs_internal.c | 12 +---
9 files changed, 103 insertions(+), 26 deletions(-)
ACK.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 6/6] build: add syntax check for proper flags use
2011/7/15 Eric Blake ebl...@redhat.com: Enforce the recent flags cleanups - we want to use 'unsigned int flags' in any of our APIs (except where backwards compatibility is important, in the public migration APIs), and that all flags are checked for validity (except when there are stub functions that completely ignore the flags argument). There are a few minor tweaks done here to avoid false positives: signed arguments passed to open() are renamed oflags, and flags arguments that are legitimately ignored are renamed flags_unused. * cfg.mk (sc_flags_usage): New rule. (exclude_file_name_regexp--sc_flags_usage): And a few exemptions. (sc_flags_debug): Tweak wording. * src/util/iohelper.c (runIO, main): Rename variable. * src/util/util.c (virSetInherit): Likewise. * src/fdstream.h (virFDStreamOpenFile, virFDStreamCreateFile): Likewise. * src/fdstream.c (virFDStreamOpenFileInternal) (virFDStreamOpenFile, virFDStreamCreateFile): Likewise. * src/util/command.c (virExecWithHook) [WIN32]: Likewise. * src/util/util.c (virFileOpenAs, virDirCreate) [WIN32]: Likewise. * src/locking/lock_manager.c (virLockManagerPluginNew) [!HAVE_DLFCN_H]: Likewise. * src/locking/lock_driver_nop.c (virLockManagerNopNew) (virLockManagerNopAddResource, virLockManagerNopAcquire) (virLockManagerNopRelease, virLockManagerNopInquire): Likewise. --- v3: minor tweaks to make syntax check rules catch a few more cases cfg.mk | 30 ++ src/fdstream.c | 28 ++-- src/fdstream.h | 6 +++--- src/locking/lock_driver_nop.c | 10 +- src/locking/lock_manager.c | 7 --- src/util/command.c | 2 +- src/util/iohelper.c | 18 +- src/util/util.c | 14 +++--- 8 files changed, 69 insertions(+), 46 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: improve option handling
2011/7/15 Eric Blake ebl...@redhat.com: The documentation for vshCommandOptString claims that it returns -1 on a missing required argument, but in reality, that error message was unreachable (it was buried inside an if clause that is true only if the argument was present). The code was so hairy that I decided a rewrite would make it easier to understand, and actually return the error values we want. In the process, this guarantees that --option '' will either return -1 or 1, depending on whether EMPTY_OK was set, reserving 0 solely for the case of an option not present and not required. Meanwhile, our construction guarantees that all vshCmdOpt have a non-null def member, so there are some redundant checks that can be trimmed. * tools/virsh.c (vshCommandOpt): Alter signature. (vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL) (vshCommandOptString, vshCommandOptLongLong) (vshCommandOptULongLong, vshCommandOptBool): Adjust all callers. --- This patch replaces the one mentioned here: https://www.redhat.com/archives/libvir-list/2011-July/msg00995.html tools/virsh.c | 302 +++-- 1 files changed, 206 insertions(+), 96 deletions(-) Looks good, ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 15/27] uml: reject unknown flags
2011/7/13 Eric Blake ebl...@redhat.com:
On 07/13/2011 06:41 AM, Matthias Bolte wrote:
2011/7/8 Eric Blake ebl...@redhat.com:
* src/uml/uml_driver.c (umlOpen, umlDomainGetXMLDesc)
(umlDomainBlockPeek): Reject unknown flags.
---
src/uml/uml_driver.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
@@ -1559,11 +1562,14 @@ cleanup:
static char *umlDomainGetXMLDesc(virDomainPtr dom,
- unsigned int flags ATTRIBUTE_UNUSED) {
+ unsigned int flags)
+{
struct uml_driver *driver = dom-conn-privateData;
virDomainObjPtr vm;
char *ret = NULL;
+ virCheckFlags(0, NULL);
+
umlDriverLock(driver);
vm = virDomainFindByUUID(driver-domains, dom-uuid);
umlDriverUnlock(driver);
Don't get fooled by the ATTRIBUTE_UNUSED again. All *DomainGetXMLDesc
use virDomainDefFormat and have to accept all flags that
virDomainDefFormat accepts. I suggest to recheck your series for this
pattern, here it's just the first time that I notice it.
Ouch. Good catch, and I'll have to fix that shortly. I'll post the
patch before I commit it, but it should be considered a trivial
regression fix, so I'll commit it without waiting for review.
ACK, with that virCheckFlags loosened correctly.
I squashed this in for this patch, and now I'm working on using the same
fix for the regression committed in all the prior pushed patches.
diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c
index 132aef1..da91687 100644
--- i/src/uml/uml_driver.c
+++ w/src/uml/uml_driver.c
@@ -1599,7 +1599,9 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom,
virDomainObjPtr vm;
char *ret = NULL;
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_UPDATE_CPU, NULL);
umlDriverLock(driver);
vm = virDomainFindByUUID(driver-domains, dom-uuid);
I'm not sure if it's a good idea to add the flags check for this set
of flags at the driver level. Maybe it should be moved one level down
into virDomainDefFormat. This avoids touching all drivers should we
ever add a new VIR_DOMAIN_XML_* flag.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
