August 2011 - Devel - libvirt List Archives

[libvirt] [PATCH v2] Ensure client streams are closed when marking a client for close
by Daniel P. Berrange 15 Aug '11

15 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> NB, previous patch was borked due to bad rebase Every active stream results in a reference being held on the virNetServerClientPtr object. This meant that if a client quit with any streams active, although all I/O was stopped the virNetServerClientPtr object would leak. This causes libvirtd to leak any file handles associated with open streams when a client quit To fix this, when we call virNetServerClientClose there is a callback invoked which lets the daemon release the streams and thus the extra references * daemon/remote.c: Add a hook to close all streams * daemon/stream.c, daemon/stream.h: Add API for releasing all streams * src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Allow registration of a hook to trigger when closing client --- daemon/remote.c | 11 ++++++++++- daemon/stream.c | 38 ++++++++++++++++++++++++++++++++------ daemon/stream.h | 3 +++ src/rpc/virnetserverclient.c | 21 +++++++++++++++++++++ src/rpc/virnetserverclient.h | 5 +++++ 5 files changed, 71 insertions(+), 7 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 939044c..0f088c6 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -439,6 +439,13 @@ static void remoteClientFreeFunc(void *data) } +static void remoteClientCloseFunc(virNetServerClientPtr client) +{ + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + daemonRemoveAllClientStreams(priv->streams); +} + int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, virNetServerClientPtr client) @@ -460,7 +467,9 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, for (i = 0 ; i < VIR_DOMAIN_EVENT_ID_LAST ; i++) priv->domainEventCallbackID[i] = -1; - virNetServerClientSetPrivateData(client, priv, remoteClientFreeFunc); + virNetServerClientSetPrivateData(client, priv, + remoteClientFreeFunc); + virNetServerClientSetCloseHook(client, remoteClientCloseFunc); return 0; } diff --git a/daemon/stream.c b/daemon/stream.c index 4a8f1ee..c6865a8 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -334,13 +334,17 @@ int daemonFreeClientStream(virNetServerClientPtr client, msg = stream->rx; while (msg) { virNetMessagePtr tmp = msg->next; - /* Send a dummy reply to free up 'msg' & unblock client rx */ - memset(msg, 0, sizeof(*msg)); - msg->header.type = VIR_NET_REPLY; - if (virNetServerClientSendMessage(client, msg) < 0) { - virNetServerClientImmediateClose(client); + if (client) { + /* Send a dummy reply to free up 'msg' & unblock client rx */ + memset(msg, 0, sizeof(*msg)); + msg->header.type = VIR_NET_REPLY; + if (virNetServerClientSendMessage(client, msg) < 0) { + virNetServerClientImmediateClose(client); + virNetMessageFree(msg); + ret = -1; + } + } else { virNetMessageFree(msg); - ret = -1; } msg = tmp; } @@ -441,6 +445,28 @@ daemonRemoveClientStream(virNetServerClientPtr client, } +void +daemonRemoveAllClientStreams(daemonClientStream *stream) +{ + daemonClientStream *tmp; + + VIR_DEBUG("stream=%p", stream); + + while (stream) { + tmp = stream->next; + + if (!stream->closed) { + virStreamEventRemoveCallback(stream->st); + virStreamAbort(stream->st); + } + + daemonFreeClientStream(NULL, stream); + + VIR_DEBUG("next stream=%p", stream); + stream = tmp; + } +} + /* * Returns: * -1 if fatal error occurred diff --git a/daemon/stream.h b/daemon/stream.h index 626ae60..7c2d8dc 100644 --- a/daemon/stream.h +++ b/daemon/stream.h @@ -45,4 +45,7 @@ int daemonRemoveClientStream(virNetServerClientPtr client, daemonClientStream *stream); +void +daemonRemoveAllClientStreams(daemonClientStream *stream); + #endif /* __LIBVIRTD_STREAM_H__ */ diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index e246fa5..a73b06d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -97,6 +97,7 @@ struct _virNetServerClient void *privateData; virNetServerClientFreeFunc privateDataFreeFunc; + virNetServerClientCloseFunc privateDataCloseFunc; }; @@ -492,6 +493,15 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client) } +void virNetServerClientSetCloseHook(virNetServerClientPtr client, + virNetServerClientCloseFunc cf) +{ + virNetServerClientLock(client); + client->privateDataCloseFunc = cf; + virNetServerClientUnlock(client); +} + + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque) @@ -560,6 +570,8 @@ void virNetServerClientFree(virNetServerClientPtr client) */ void virNetServerClientClose(virNetServerClientPtr client) { + virNetServerClientCloseFunc cf; + virNetServerClientLock(client); VIR_DEBUG("client=%p refs=%d", client, client->refs); if (!client->sock) { @@ -567,6 +579,15 @@ void virNetServerClientClose(virNetServerClientPtr client) return; } + if (client->privateDataCloseFunc) { + cf = client->privateDataCloseFunc; + client->refs++; + virNetServerClientUnlock(client); + (cf)(client); + virNetServerClientLock(client); + client->refs--; + } + /* Do now, even though we don't close the socket * until end, to ensure we don't get invoked * again due to tls shutdown */ diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3d2e1fb..bedb179 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -82,6 +82,11 @@ void virNetServerClientSetPrivateData(virNetServerClientPtr client, virNetServerClientFreeFunc ff); void *virNetServerClientGetPrivateData(virNetServerClientPtr client); +typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client); + +void virNetServerClientSetCloseHook(virNetServerClientPtr client, + virNetServerClientCloseFunc cf); + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque); -- 1.7.6

2 1

[libvirt] [PATCH 0/4 v2] macvtap: Support for sending port profile message for a SRIOV VF to its PF
by Roopa Prabhu 15 Aug '11

15 Aug '11

This patch tries to fix getPhysFn in macvtap.c to get the physical function(PF) of the direct attach interface, if the interface is a SR-IOV VF. It moves some of the sriov pci device handling code from node_device_driver to src/util/pci.[ch]. This patch series implements the following 01/3 - pci: Move some pci sriov helper code out of node device driver to util/pci 02/3 - pci: Add helper functions for sriov devices 02/3 - interface: Add functions to get sriov PF/VF relationship of a net interface 03/3 - macvtap: Fix getPhysfn to get the PF of a direct attach network interface Changelog: ---------- v1: RFC patch introduced new functions to get PF/VF relationship of SRIOV net devices by comparing PF/VF sysfs device links. Also mentioned the existance of some related code in node_device_driver. The feedback was to move and use the node_device_driver code v2: Moves sriov get_physical_function and get_virtual_functions from node_device_driver to src/util/pci*. And reworks the rest of the patches around the new code. Signed-off-by: Roopa Prabhu <roprabhu(a)cisco.com> Signed-off-by: Christian Benvenuti <benve(a)cisco.com> Signed-off-by: David Wang <dwang2(a)cisco.com>

2 13

[libvirt] [PATCH] Ensure client streams are closed when marking a client for close
by Daniel P. Berrange 15 Aug '11

15 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> Every active stream results in a reference being held on the virNetServerClientPtr object. This meant that if a client quit with any streams active, although all I/O was stopped the virNetServerClientPtr object would leak. This causes libvirtd to leak any file handles associated with open streams when a client quit To fix this, when we call virNetServerClientClose there is a callback invoked which lets the daemon release the streams and thus the extra references * daemon/remote.c: Add a hook to close all streams * src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Allow registration of a hook to trigger when closing client --- daemon/remote.c | 13 ++++++++++--- src/rpc/virnetserverclient.c | 21 +++++++++++++++++++++ src/rpc/virnetserverclient.h | 5 +++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ec261e2..0f088c6 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -419,8 +419,6 @@ static void remoteClientFreeFunc(void *data) { struct daemonClientPrivate *priv = data; - daemonRemoveAllClientStreams(priv->streams); - /* Deregister event delivery callback */ if (priv->conn) { int i; @@ -441,6 +439,13 @@ static void remoteClientFreeFunc(void *data) } +static void remoteClientCloseFunc(virNetServerClientPtr client) +{ + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + daemonRemoveAllClientStreams(priv->streams); +} + int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, virNetServerClientPtr client) @@ -462,7 +467,9 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, for (i = 0 ; i < VIR_DOMAIN_EVENT_ID_LAST ; i++) priv->domainEventCallbackID[i] = -1; - virNetServerClientSetPrivateData(client, priv, remoteClientFreeFunc); + virNetServerClientSetPrivateData(client, priv, + remoteClientFreeFunc); + virNetServerClientSetCloseHook(client, remoteClientCloseFunc); return 0; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index e246fa5..a73b06d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -97,6 +97,7 @@ struct _virNetServerClient void *privateData; virNetServerClientFreeFunc privateDataFreeFunc; + virNetServerClientCloseFunc privateDataCloseFunc; }; @@ -492,6 +493,15 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client) } +void virNetServerClientSetCloseHook(virNetServerClientPtr client, + virNetServerClientCloseFunc cf) +{ + virNetServerClientLock(client); + client->privateDataCloseFunc = cf; + virNetServerClientUnlock(client); +} + + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque) @@ -560,6 +570,8 @@ void virNetServerClientFree(virNetServerClientPtr client) */ void virNetServerClientClose(virNetServerClientPtr client) { + virNetServerClientCloseFunc cf; + virNetServerClientLock(client); VIR_DEBUG("client=%p refs=%d", client, client->refs); if (!client->sock) { @@ -567,6 +579,15 @@ void virNetServerClientClose(virNetServerClientPtr client) return; } + if (client->privateDataCloseFunc) { + cf = client->privateDataCloseFunc; + client->refs++; + virNetServerClientUnlock(client); + (cf)(client); + virNetServerClientLock(client); + client->refs--; + } + /* Do now, even though we don't close the socket * until end, to ensure we don't get invoked * again due to tls shutdown */ diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3d2e1fb..bedb179 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -82,6 +82,11 @@ void virNetServerClientSetPrivateData(virNetServerClientPtr client, virNetServerClientFreeFunc ff); void *virNetServerClientGetPrivateData(virNetServerClientPtr client); +typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client); + +void virNetServerClientSetCloseHook(virNetServerClientPtr client, + virNetServerClientCloseFunc cf); + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque); -- 1.7.6

1 0

[libvirt] [PATCH] qemu: error message should show uri instead of (null)
by Peter Krempa 15 Aug '11

15 Aug '11

Fix pointer for error message uri if domain migration fails. BZ# 730244 --- src/qemu/qemu_migration.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..d900020 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2079,7 +2079,7 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, qemuDomainObjExitRemoteWithDriver(driver, vm); if (dconn == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("Failed to connect to remote libvirt URI %s"), uri); + _("Failed to connect to remote libvirt URI %s"), dconnuri); return -1; } -- 1.7.3.4

5 4

[libvirt] [PATCH] libvirtd.init.in: stop/restart() - wrong return value in case of fail
by Douglas Schilling Landgraf 15 Aug '11

15 Aug '11

Currently the function stop() always return 0 (OK) from killproc() even in case of error. Signed-off-by: Douglas Schilling Landgraf <dougsland(a)redhat.com> --- daemon/libvirtd.init.in | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in index aa7870c..0697a2b 100644 --- a/daemon/libvirtd.init.in +++ b/daemon/libvirtd.init.in @@ -76,6 +76,8 @@ stop() { rm -f @localstatedir@/lock/subsys/$SERVICE rm -f $PIDFILE rm -rf @localstatedir@/cache/libvirt/* + else + exit $RETVAL fi } -- 1.7.1

2 1

[libvirt] [PATCH] daemon: Fix regression of libvirtd reloading support
by Osier Yang 15 Aug '11

15 Aug '11

This is introduced by commit df0b57a95a, which forgot to add signal handler for SIGHUP. A simple reproduce method: 1) Create a domain XML under /etc/libvirt/qemu 2) % kill -SIGHUP $(pidof libvirtd) 3) % virsh list --all (the new created domain XML is not listed) --- daemon/libvirtd.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b866a01..3e0159b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1102,6 +1102,17 @@ static void daemonShutdownHandler(virNetServerPtr srv, virNetServerQuit(srv); } +static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, + siginfo_t *sig ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + VIR_INFO(_("Reloading configuration on SIGHUP")); + virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", + VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL); + if (virStateReload() < 0) + VIR_WARN("Error while reloading drivers"); +} + static int daemonSetupSignals(virNetServerPtr srv) { if (virNetServerAddSignalHandler(srv, SIGINT, daemonShutdownHandler, NULL) < 0) @@ -1110,6 +1121,8 @@ static int daemonSetupSignals(virNetServerPtr srv) return -1; if (virNetServerAddSignalHandler(srv, SIGTERM, daemonShutdownHandler, NULL) < 0) return -1; + if (virNetServerAddSignalHandler(srv, SIGHUP, daemonReloadHandler, NULL) < 0) + return -1; return 0; } -- 1.7.6

2 2

[libvirt] snapshots += domain description?
by Philipp Hahn 15 Aug '11

15 Aug '11

Hello, I just encountered a problem with the current snapshot implementation for qemu/kvm: To revert back to a snapshot, the domain description must closely match the domain description when the snapshot was created. If the ram-size doesn't match or the VM now contains fewer CPUs or contains additional disks, kvm will either not load the snapshot or the machine will be broken afterwarts. Is this a known problem? Without looking into the implementation details I think saving the full domain description instead of just a reference to the domain uuid as part of the snapshot description would work better. Or am I supposed to save the domain description myself in addition the the data saved in /var/lib/libvirt/qemu/snapshot/$DOMAIN_NAME/$SNAPSHOT_NAME.xml. I think this is very important feature, because when you - for example - notice that you need additional disk space and would like to include an additional block device, this is your ideal time to take a snapshot you can revert to when you do something wrong. Looking at VMware I see that reverting a VM there will not only revert the disks and ram back to the saved state, but also the description including name, ram size, etc. Any comment or advise is appreciated. Sincerely Philipp Hahn -- Philipp Hahn Open Source Software Engineer hahn(a)univention.de Univention GmbH Linux for Your Business fon: +49 421 22 232- 0 Mary-Somerville-Str.1 28359 Bremen fax: +49 421 22 232-99 http://www.univention.de

2 13

[libvirt] [PATCH] maint: let emacs avoid tabs in rng files
by Eric Blake 13 Aug '11

13 Aug '11

* .dir-locals.el: Add nxml-mode preferences. --- I'm getting tired of accidentally adding tabs when I touch .rng files, so I'm pushing this under the trivial rule. .dir-locals.el | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/.dir-locals.el b/.dir-locals.el index d5eba86..c10603c 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -11,4 +11,7 @@ (sh-mode . ( (indent-tabs-mode . nil) )) + (nxml-mode . ( + (indent-tabs-mode . nil) + )) ) -- 1.7.4.4

1 0

[libvirt] [PATCH] doc: Document the Block Job API
by Adam Litke 13 Aug '11

13 Aug '11

Hi DV, This patch adds some basic documentation to the development guide for the Block Job APIs as you requested. Also included is a sample program that shows end-to-end use of the BlockPull operation. I hope this is close to what you had in mind. Signed-off-by: Adam Litke <agl(a)us.ibm.com> --- en-US/Guest_Domains.xml | 154 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 154 insertions(+), 0 deletions(-) diff --git a/en-US/Guest_Domains.xml b/en-US/Guest_Domains.xml index 0324e78..202881d 100644 --- a/en-US/Guest_Domains.xml +++ b/en-US/Guest_Domains.xml @@ -1219,6 +1219,160 @@ fprintf(stderr, "Device is suitable for passthrough to a guest\n"); </section> + <section id="Application_Development_Guide-Live_Config-Block_Jobs"> + <title>Block Device Jobs</title> + + <para> + Libvirt provides a generic Block Job API that can be used to initiate + and manage operations on disks that belong to a domain. Jobs are + started by calling the function associated with the desired operation + (eg. <literal>virDomainBlockPull</literal>). Once started, all block + jobs are managed in the same manner. They can be aborted, throttled, + and queried. Upon completion, an asynchronous event is issued to + indicate the final status. + </para> + + <para> + The following block jobs can be started: + </para> + <orderedlist> + <listitem> + <para> + <literal>virDomainBlockPull()</literal> starts a block pull + operation for the specified disk. This operation is valid only for + specially configured disks. BlockPull will populate a disk image + with data from its backing image. Once all data from its backing + image has been pulled, the disk no longer depends on a backing + image. + </para> + </listitem> + </orderedlist> + + <para> + A disk can be queried for active block jobs by using + <literal>virDomainGetBlockJobInfo()</literal>. If found, job + information is reported in a structure that contains: the job type, + bandwidth throttling setting, and progress information. + </para> + + <para> + <literal>virDomainBlockJobAbort()</literal> can be used to cancel the + active block job on the specified disk. + </para> + + <para> + Use <literal>virDomainBlockJobSetSpeed()</literal> to limit the amount + of bandwidth that a block job may consume. Bandwidth is specified in + units of MB/sec. + </para> + + <para> + When a block job operation completes, the final status is reported using + an asynchronous event. To receive this event, register a + <literal>virConnectDomainEventBlockJobCallback</literal> function which + will receive the disk, event type, and status as parameters. + </para> + + <programlisting> +<![CDATA[/* example blockpull-example.c */ +/* compile with: gcc -g -Wall blockpull-example.c -o blockpull-example -lvirt */ +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <libvirt/libvirt.h> + +int do_cmd(const char *cmdline) +{ + int status = system(cmdline); + if (status < 0) + return -1; + else + return WEXITSTATUS(status); +} + +virDomainPtr make_domain(virConnectPtr conn) +{ + virDomainPtr dom; + char domxml[] = \ + "<domain type='kvm'> \ + <name>example</name> \ + <memory>131072</memory> \ + <vcpu>1</vcpu> \ + <os> \ + <type arch='x86_64' machine='pc-0.13'>hvm</type> \ + </os> \ + <devices> \ + <disk type='file' device='disk'> \ + <driver name='qemu' type='qed'/> \ + <source file='/var/lib/libvirt/images/example.qed' /> \ + <target dev='vda' bus='virtio'/> \ + </disk> \ + </devices> \ + </domain>"; + + do_cmd("qemu-img create -f raw /var/lib/libvirt/images/backing.qed 100M"); + do_cmd("qemu-img create -f qed -b /var/lib/libvirt/images/backing.qed \ + /var/lib/libvirt/images/example.qed"); + + dom = virDomainCreateXML(conn, domxml, 0); + return dom; +} + +int main(int argc, char *argv[]) +{ + virConnectPtr conn; + virDomainPtr dom = NULL; + char disk[] = "/var/lib/libvirt/images/example.qed"; + + conn = virConnectOpen("qemu:///system"); + if (conn == NULL) { + fprintf(stderr, "Failed to open connection to qemu:///system\n"); + goto error; + } + + dom = make_domain(conn); + if (dom == NULL) { + fprintf(stderr, "Failed to create domain\n"); + goto error; + } + + if ((virDomainBlockPull(dom, disk, 0, 0)) < 0) { + fprintf(stderr, "Failed to start block pull"); + goto error; + } + + while (1) { + virDomainBlockJobInfo info; + int ret = virDomainGetBlockJobInfo(dom, disk, &info, 0); + + if (ret == 1) { + printf("BlockPull progress: %0.0f %%\n", + (float)(100 * info.cur / info.end)); + } else if (ret == 0) { + printf("BlockPull complete\n"); + break; + } else { + fprintf(stderr, "Failed to query block jobs\n"); + break; + } + usleep(100000); + } + +error: + unlink("/var/lib/libvirt/images/backing.qed"); + unlink("/var/lib/libvirt/images/example.qed"); + if (dom != NULL) { + virDomainDestroy(dom); + virDomainFree(dom); + } + if (conn != NULL) + virConnectClose(conn); + return 0; +}]]> + </programlisting> + + </section> + </section> <section id="Application_Development_Guide-Guest_Domains-Security"> -- 1.7.3

2 1

[libvirt] KVM Forum 2011
by KVM Forum 2011 PC 13 Aug '11

13 Aug '11

For those who are travelling to the KVM Forum, looking forward to seeing you in Vancouver. And for those who can't make it, we'll be posting the presentations online as well as recording the presentations and making the videos available online (subject to presenter's approval). Here is the schedule if you haven't seen it already. http://www.linux-kvm.org/page/KVM_Forum_2011#Schedule thanks, -KVM Forum 2011 PC

1 0