[libvirt] [PATCH v2] esx: Fix and improve esxListAllDomains function
by Matthias Bolte
Avoid requesting information such as identity or power state when it
is not necessary.
Lookup virtual machine list with the required fields (configStatus,
name, and config.uuid) to make esxVI_GetVirtualMachineIdentity work.
No need to call esxVI_GetNumberOfSnapshotTrees. rootSnapshotTreeList
can be tested for emptiness by checking it for NULL.
esxVI_LookupRootSnapshotTreeList already does the error reporting,
don't overwrite it.
Check if autostart is enabled at all before looking up the individual
autostart setting of a virtual machine.
Reorder VIR_EXPAND_N(doms, ndoms, 1) to avoid leaking the result of
the call to virGetDomain if VIR_EXPAND_N fails.
Replace VIR_EXPAND_N by VIR_RESIZE_N to avoid quadratic scaling, as in
the Hyper-V version of the function.
If virGetDomain fails it already reports an error, don't overwrite it
with an OOM error.
All items in doms up to the count-th one are valid, no need to double
check before freeing them.
Finally, don't leak autoStartDefaults and powerInfoList.
---
v2: Replace VIR_EXPAND_N by VIR_RESIZE_N to avoid quadratic scaling,
as in the Hyper-V version of the function.
src/esx/esx_driver.c | 116 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 76 insertions(+), 40 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 991f03c..cf0551f 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5008,13 +5008,15 @@ esxListAllDomains(virConnectPtr conn,
{
int ret = -1;
esxPrivate *priv = conn->privateData;
+ bool needIdentity;
+ bool needPowerState;
virDomainPtr dom;
virDomainPtr *doms = NULL;
size_t ndoms = 0;
+ esxVI_String *propertyNameList = NULL;
esxVI_ObjectContent *virtualMachineList = NULL;
esxVI_ObjectContent *virtualMachine = NULL;
- esxVI_String *propertyNameList = NULL;
- esxVI_AutoStartDefaults *autostart_defaults = NULL;
+ esxVI_AutoStartDefaults *autoStartDefaults = NULL;
esxVI_VirtualMachinePowerState powerState;
esxVI_AutoStartPowerInfo *powerInfoList = NULL;
esxVI_AutoStartPowerInfo *powerInfo = NULL;
@@ -5023,7 +5025,6 @@ esxListAllDomains(virConnectPtr conn,
int id;
unsigned char uuid[VIR_UUID_BUFLEN];
int count = 0;
- int snapshotCount;
bool autostart;
int state;
@@ -5033,7 +5034,7 @@ esxListAllDomains(virConnectPtr conn,
* - persistence: all esx machines are persistent
* - managed save: esx doesn't support managed save
*/
- if ((MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) &&
+ if ((MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) &&
!MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT)) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) &&
!MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE))) {
@@ -5045,23 +5046,49 @@ esxListAllDomains(virConnectPtr conn,
goto cleanup;
}
- if (esxVI_EnsureSession(priv->primary) < 0)
+ if (esxVI_EnsureSession(priv->primary) < 0)
return -1;
/* check system default autostart value */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART)) {
if (esxVI_LookupAutoStartDefaults(priv->primary,
- &autostart_defaults) < 0)
+ &autoStartDefaults) < 0) {
+ goto cleanup;
+ }
+
+ if (autoStartDefaults->enabled == esxVI_Boolean_True) {
+ if (esxVI_LookupAutoStartPowerInfoList(priv->primary,
+ &powerInfoList) < 0) {
+ goto cleanup;
+ }
+ }
+ }
+
+ needIdentity = MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT) ||
+ domains != NULL;
+
+ if (needIdentity) {
+ /* Request required data for esxVI_GetVirtualMachineIdentity */
+ if (esxVI_String_AppendValueListToList(&propertyNameList,
+ "configStatus\0"
+ "name\0"
+ "config.uuid\0") < 0) {
goto cleanup;
+ }
+ }
+
+ needPowerState = MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) ||
+ MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE) ||
+ domains != NULL;
- if (esxVI_LookupAutoStartPowerInfoList(priv->primary,
- &powerInfoList) < 0)
+ if (needPowerState) {
+ if (esxVI_String_AppendValueToList(&propertyNameList,
+ "runtime.powerState") < 0) {
goto cleanup;
+ }
}
- if (esxVI_String_AppendValueToList(&propertyNameList,
- "runtime.powerState") < 0 ||
- esxVI_LookupVirtualMachineList(priv->primary, propertyNameList,
+ if (esxVI_LookupVirtualMachineList(priv->primary, propertyNameList,
&virtualMachineList) < 0)
goto cleanup;
@@ -5073,12 +5100,21 @@ esxListAllDomains(virConnectPtr conn,
for (virtualMachine = virtualMachineList; virtualMachine != NULL;
virtualMachine = virtualMachine->_next) {
+ if (needIdentity) {
+ VIR_FREE(name);
- VIR_FREE(name);
+ if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id,
+ &name, uuid) < 0) {
+ goto cleanup;
+ }
+ }
- if (esxVI_GetVirtualMachineIdentity(virtualMachine, &id, &name, uuid) < 0 ||
- esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0)
- goto cleanup;
+ if (needPowerState) {
+ if (esxVI_GetVirtualMachinePowerState(virtualMachine,
+ &powerState) < 0) {
+ goto cleanup;
+ }
+ }
/* filter by active state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) &&
@@ -5090,23 +5126,17 @@ esxListAllDomains(virConnectPtr conn,
/* filter by snapshot existence */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)) {
+ esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList);
+
if (esxVI_LookupRootSnapshotTreeList(priv->primary, uuid,
&rootSnapshotTreeList) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Couldn't retrieve snapshot list for "
- "domain '%s'"), name);
goto cleanup;
}
- snapshotCount = esxVI_GetNumberOfSnapshotTrees(rootSnapshotTreeList,
- true, false);
-
- esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList);
-
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) &&
- snapshotCount > 0) ||
+ rootSnapshotTreeList != NULL) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) &&
- snapshotCount == 0)))
+ rootSnapshotTreeList == NULL)))
continue;
}
@@ -5114,19 +5144,18 @@ esxListAllDomains(virConnectPtr conn,
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART)) {
autostart = false;
- for (powerInfo = powerInfoList; powerInfo != NULL;
- powerInfo = powerInfo->_next) {
- if (STREQ(powerInfo->key->value, virtualMachine->obj->value)) {
- if (STRCASEEQ(powerInfo->startAction, "powerOn"))
- autostart = true;
+ if (autoStartDefaults->enabled == esxVI_Boolean_True) {
+ for (powerInfo = powerInfoList; powerInfo != NULL;
+ powerInfo = powerInfo->_next) {
+ if (STREQ(powerInfo->key->value, virtualMachine->obj->value)) {
+ if (STRCASEEQ(powerInfo->startAction, "powerOn"))
+ autostart = true;
- break;
+ break;
+ }
}
}
- autostart = autostart &&
- autostart_defaults->enabled == esxVI_Boolean_True;
-
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) &&
autostart) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART) &&
@@ -5137,6 +5166,7 @@ esxListAllDomains(virConnectPtr conn,
/* filter by domain state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) {
state = esxVI_VirtualMachinePowerState_ConvertToLibvirt(powerState);
+
if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_RUNNING) &&
state == VIR_DOMAIN_RUNNING) ||
(MATCH(VIR_CONNECT_LIST_DOMAINS_PAUSED) &&
@@ -5156,17 +5186,18 @@ esxListAllDomains(virConnectPtr conn,
continue;
}
- if (!(dom = virGetDomain(conn, name, uuid)))
+ if (VIR_RESIZE_N(doms, ndoms, count, 2) < 0)
goto no_memory;
+ if (!(dom = virGetDomain(conn, name, uuid)))
+ goto cleanup;
+
/* Only running/suspended virtual machines have an ID != -1 */
if (powerState != esxVI_VirtualMachinePowerState_PoweredOff)
dom->id = id;
else
dom->id = -1;
- if (VIR_EXPAND_N(doms, ndoms, 1) < 0)
- goto no_memory;
doms[count++] = dom;
}
@@ -5178,14 +5209,19 @@ esxListAllDomains(virConnectPtr conn,
cleanup:
if (doms) {
for (id = 0; id < count; id++) {
- if (doms[id])
- virDomainFree(doms[id]);
+ virDomainFree(doms[id]);
}
+
+ VIR_FREE(doms);
}
- VIR_FREE(doms);
+
VIR_FREE(name);
+ esxVI_AutoStartDefaults_Free(&autoStartDefaults);
+ esxVI_AutoStartPowerInfo_Free(&powerInfoList);
esxVI_String_Free(&propertyNameList);
esxVI_ObjectContent_Free(&virtualMachineList);
+ esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList);
+
return ret;
no_memory:
--
1.7.4.1
12 years, 1 month
[libvirt] TSC scaling interface to management
by Marcelo Tosatti
HW TSC scaling is a feature of AMD processors that allows a
multiplier to be specified to the TSC frequency exposed to the guest.
KVM also contains provision to trap TSC ("KVM: Infrastructure for
software and hardware based TSC rate scaling" cc578287e3224d0da)
or advance TSC frequency.
This is useful when migrating to a host with different frequency and
the guest is possibly using direct RDTSC instructions for purposes
other than measuring cycles (that is, it previously calculated
cycles-per-second, and uses that information which is stale after
migration).
"qemu-x86: Set tsc_khz in kvm when supported" (e7429073ed1a76518)
added support for tsc_khz= option in QEMU.
I am proposing the following changes so that management applications
can work with this:
1) New option for tsc_khz, which is tsc_khz=host (QEMU command line
option). Host means that QEMU is responsible for retrieving the
TSC frequency of the host processor and use that.
Management application does not have to deal with the burden.
2) New subsection with tsc_khz value. Destination host should consult
supported features of running kernel and fail if feature is unsupported.
It is not necessary to use this tsc_khz setting with modern guests
using paravirtual clocks, or when its known that applications make
proper use of the time interface provided by operating systems.
On the other hand, legacy applications or setups which require no
modification and correct operation while virtualized and make
use of RDTSC might need this.
Therefore it appears that this "tsc_khz=auto" option can be specified
only if the user specifies so (it can be a per-guest flag hidden
in the management configuration/manual).
Sending this email to gather suggestions (or objections)
to this interface.
12 years, 1 month
[libvirt] Patch to default selinuxfs mount point to /sys/fs/selinux
by Daniel J Walsh
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Currently if you build on a machine that does not support SELinux we end up
with the default mount point being /selinux, since this is moved to
/sys/fs/selinux, we should start defaulting there.
I believe this is causing a bug in libvirt-lxc when /selinux does not exists,
even though /sys/fs/selinux exists.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iEYEARECAAYFAlBl6F8ACgkQrlYvE4MpobPz0ACgxPdENx/KDtQY7YGT7BDXoLP3
AVIAoJwUvTib72U0VJ3dnVoGU0PYjMXZ
=XU8L
-----END PGP SIGNATURE-----
12 years, 1 month
[libvirt] virsh --connect not working
by hiren panchasara
I've installed libvirt-0.9.13 from ports on my freebsd machine.
I started libvirtd:
$ ps awwux | grep libvirtd
root 11470 0.0 0.4 103100 31948 - I 10:41PM 0:00.35
libvirtd -v -d
$ sudo virsh --connect qemu:///system
error: no connection driver available for No connection for URI
qemu:///system
error: failed to connect to the hypervisor
I am sure I am missing something obvious.
Thanks in advance,
Hiren
12 years, 1 month
[libvirt] [PATCH] Fix QEMU test with 1.2.0 help output
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The help output for QEMU 1.2.0 changed 'pci-assign' to 'kvm-pci-assign'.
Since the new capabilities code does exact device name matching
instead of substring matching, this caused the capabilities to go
missing.
Pushed as a build break fix
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f0442ee..cac4917 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1321,6 +1321,8 @@ static struct qemuCapsObjectTypeProps qemuCapsObjectProps[] = {
ARRAY_CARDINALITY(qemuCapsObjectPropsVirtioNet) },
{ "pci-assign", qemuCapsObjectPropsPciAssign,
ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) },
+ { "kvm-pci-assign", qemuCapsObjectPropsPciAssign,
+ ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) },
{ "scsi-disk", qemuCapsObjectPropsScsiDisk,
ARRAY_CARDINALITY(qemuCapsObjectPropsScsiDisk) },
{ "ide-drive", qemuCapsObjectPropsIDEDrive,
--
1.7.11.4
12 years, 1 month
[libvirt] [PATCH 0/9] Use the systemd journal for logging
by Daniel P. Berrange
The systemd journal provides a highly efficient way for apps
to generate structured log messages. Since systemd will be
used by default on future Fedora/RHEL distros (and likely
many others), we should take advantage of the journal out of
the box for improved logging capabilities.
Amongst other things, this allows very easy browsing of the
libvirt logs after the fact, filtering on filename / line
number / function name and more. This is a big benefit
to debugging, compared to current case where we need to
decide the filters when starting libvirtd. For example
it becomes possible to configure libvirt to include all
QEMU logging, and then later ask the journal for only
messages related to the QEMU json monitor.
12 years, 1 month
[libvirt] [PATCH v3] Fix dnsmasq/radvd on bridges not being able to be bound to IPv6 address on "recent" kernels
by Benjamin Cama
Hi,
I hit this problem recently when trying to create a bridge with an IPv6
address on a 3.2 kernel: dnsmasq (and, further, radvd) would not bind to
the given address, waiting 20s and then giving up with -EADDRNOTAVAIL
(resp. exiting immediately with "error parsing or activating the config
file", without libvirt noticing it, BTW). This can be reproduced with (I
think) any kernel >= 2.6.39 and the following XML (to be used with
"virsh net-create"):
<network>
<name>test-bridge</name>
<bridge name='testbr0' />
<ip family='ipv6' address='fd00::1' prefix='64'>
</ip>
</network>
(it happens even when you have an IPv4, too)
The problem is that since commit [1] (which, ironically, was made to
“help IPv6 autoconfiguration”) the linux bridge code makes bridges
behave like “real” devices regarding carrier detection. This makes the
bridges created by libvirt, which are started without any up devices,
stay with the NO-CARRIER flag set, and thus prevents DAD (Duplicate
address detection) from happening, thus letting the IPv6 address flagged
as “tentative”. Such addresses cannot be bound to (see RFC 2462), so
dnsmasq fails binding to it (for radvd, it detects that "interface XXX
is not RUNNING", thus that "interface XXX does not exist, ignoring the
interface" (sic)). It seems that this behavior was enhanced somehow with
commit [2] by avoiding setting NO-CARRIER on empty bridges, but I
couldn't reproduce this behavior on my kernel. Anyway, with the “dummy
tap to set MAC address” trick, this wouldn't work.
To fix this, the idea is to get the bridge's attached device to be up so
that DAD can happen (deactivating DAD altogether is not a good idea, I
think). Currently, libvirt creates a dummy TAP device to set the MAC
address of the bridge, keeping it down. But even if we set this device
up, it is not RUNNING as soon as the tap file descriptor attached to it
is closed, thus still preventing DAD. So, we must modify the API a bit,
so that we can get the fd, keep the tap device persistent, run the
daemons, and close it after DAD has taken place. After that, the bridge
will be flagged NO-CARRIER again, but the daemons will be running, even
if not happy about the device's state (but we don't really care about
the bridge's daemons doing anything when no up interface is connected to
it).
Other solutions that I envisioned were:
* Keeping the *-nic interface up: this would waste an fd for each
bridge during all its life. May be acceptable, I don't really
know.
* Stop using the dummy tap trick, and set the MAC address directly
on the bridge: it is possible since quite some time it seems,
even if then there is the problem of the bridge not being
RUNNING when empty, contrary to what [2] says, so this will need
fixing (and this fix only happened in 3.1, so it wouldn't work
for 2.6.39)
* Using the --interface option of dnsmasq, but I saw somewhere
that it's not used by libvirt for backward compatibility. I am
not sure this would solve this problem, though, as I don't know
how dnsmasq binds itself to it with this option.
This is why this patch does what's described earlier.
This patch also makes radvd start even if the interface is
“missing” (i.e. it is not RUNNING), as it daemonizes before binding to
it, and thus sometimes does it after the interface has been brought down
by us (by closing the tap fd), and then originally stops. This also
makes it stop yelling about it in the logs when the interface is down at
a later time.
[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=1...
[2]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=b...
---
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6e260f7..8d6abd3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -65,6 +65,7 @@
#include "virnetdevtap.h"
#include "virnetdevvportprofile.h"
#include "virdbus.h"
+#include "virfile.h"
#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -987,12 +988,15 @@ networkRadvdConfContents(virNetworkObjPtr network, char **configstr)
*configstr = NULL;
- /* create radvd config file appropriate for this network */
+ /* create radvd config file appropriate for this network;
+ * IgnoreIfMissing allows radvd to start even when the bridge is down
+ */
virBufferAsprintf(&configbuf, "interface %s\n"
"{\n"
" AdvSendAdvert on;\n"
" AdvManagedFlag off;\n"
" AdvOtherConfigFlag off;\n"
+ " IgnoreIfMissing on;\n"
"\n",
network->def->bridge);
@@ -2061,6 +2065,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
virErrorPtr save_err = NULL;
virNetworkIpDefPtr ipdef;
char *macTapIfName = NULL;
+ int tapfd = -1;
/* Check to see if any network IP collides with an existing route */
if (networkCheckRouteCollision(network) < 0)
@@ -2082,10 +2087,13 @@ networkStartNetworkVirtual(struct network_driver *driver,
virReportOOMError();
goto err0;
}
+ /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */
if (virNetDevTapCreateInBridgePort(network->def->bridge,
&macTapIfName, &network->def->mac,
- NULL, NULL, NULL, NULL,
- VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) {
+ NULL, &tapfd, NULL, NULL,
+ VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
+ |VIR_NETDEV_TAP_CREATE_IFUP
+ |VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
VIR_FREE(macTapIfName);
goto err0;
}
@@ -2149,6 +2157,15 @@ networkStartNetworkVirtual(struct network_driver *driver,
if (v6present && networkStartRadvd(network) < 0)
goto err4;
+ /* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
+ * bridge's IPv6 address, so we can now set the dummy tun down.
+ */
+ if (tapfd >= 0) {
+ if (virNetDevSetOnline(macTapIfName, false) < 0)
+ goto err4;
+ VIR_FORCE_CLOSE(tapfd);
+ }
+
if (virNetDevBandwidthSet(network->def->bridge, network->def->bandwidth) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot set bandwidth limits on %s"),
@@ -2187,6 +2204,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
save_err = virSaveLastError();
if (macTapIfName) {
+ if (tapfd >= 0)
+ VIR_FORCE_CLOSE(tapfd);
ignore_value(virNetDevTapDelete(macTapIfName));
VIR_FREE(macTapIfName);
}
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index a317bcc..765c5c2 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -142,7 +142,8 @@ umlConnectTapDevice(virConnectPtr conn,
vm->uuid, NULL,
virDomainNetGetActualVirtPortProfile(net),
virDomainNetGetActualVlan(net),
- VIR_NETDEV_TAP_CREATE_IFUP) < 0) {
+ VIR_NETDEV_TAP_CREATE_IFUP
+ |VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
if (template_ifname)
VIR_FREE(net->ifname);
goto error;
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 9f2dca1..0eadd6c 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -112,18 +112,20 @@ virNetDevProbeVnetHdr(int tapfd)
*
* VIR_NETDEV_TAP_CREATE_VNET_HDR
* - Enable IFF_VNET_HDR on the tap device
+ * VIR_NETDEV_TAP_CREATE_PERSIST
+ * - The device will persist after the file descriptor is closed
*
* Creates a tap interface.
- * If the @tapfd parameter is supplied, the open tap device file
- * descriptor will be returned, otherwise the TAP device will be made
- * persistent and closed. The caller must use virNetDevTapDelete to
- * remove a persistent TAP devices when it is no longer needed.
+ * If the @tapfd parameter is supplied, the open tap device file descriptor
+ * will be returned, otherwise the TAP device will be closed. The caller must
+ * use virNetDevTapDelete to remove a persistent TAP device when it is no
+ * longer needed.
*
* Returns 0 in case of success or -1 on failure.
*/
int virNetDevTapCreate(char **ifname,
int *tapfd,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
int fd;
struct ifreq ifr;
@@ -160,7 +162,7 @@ int virNetDevTapCreate(char **ifname,
goto cleanup;
}
- if (!tapfd &&
+ if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) &&
(errno = ioctl(fd, TUNSETPERSIST, 1))) {
virReportSystemError(errno,
_("Unable to set tap device %s to persistent"),
@@ -261,14 +263,16 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
* - Enable IFF_VNET_HDR on the tap device
* VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
* - Set this interface's MAC as the bridge's MAC address
+ * VIR_NETDEV_TAP_CREATE_PERSIST
+ * - The device will persist after the file descriptor is closed
*
* This function creates a new tap device on a bridge. @ifname can be either
* a fixed name or a name template with '%d' for dynamic name allocation.
* in either case the final name for the bridge will be stored in @ifname.
- * If the @tapfd parameter is supplied, the open tap device file
- * descriptor will be returned, otherwise the TAP device will be made
- * persistent and closed. The caller must use virNetDevTapDelete to remove
- * a persistent TAP devices when it is no longer needed.
+ * If the @tapfd parameter is supplied, the open tap device file descriptor
+ * will be returned, otherwise the TAP device will be closed. The caller must
+ * use virNetDevTapDelete to remove a persistent TAP device when it is no
+ * longer needed.
*
* Returns 0 in case of success or -1 on failure
*/
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index fa6a647..980db61 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -43,6 +43,8 @@ typedef enum {
VIR_NETDEV_TAP_CREATE_VNET_HDR = 1 << 1,
/* Set this interface's MAC as the bridge's MAC address */
VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE = 1 << 2,
+ /* The device will persist after the file descriptor is closed */
+ VIR_NETDEV_TAP_CREATE_PERSIST = 1 << 3,
} virNetDevTapCreateFlags;
int virNetDevTapCreateInBridgePort(const char *brname,
--
Benjamin Cama <benjamin.cama(a)telecom-bretagne.eu>
12 years, 1 month
[libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
In the cgroups APIs we have a virCgroupKillPainfully function
which does the loop sending SIGTERM, then SIGKILL and waiting
for the process to exit. There is similar functionality for
simple processes in qemuProcessKill, but it is tangled with
the QEMU code. Untangle it to provide a virProcessKillPainfuly
function
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 8 ++---
src/qemu/qemu_process.c | 79 ++++++++----------------------------------------
src/util/virprocess.c | 57 ++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 2 ++
5 files changed, 76 insertions(+), 71 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4635a4d..dab607a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1708,6 +1708,7 @@ virPidFileDeletePath;
# virprocess.h
virProcessAbort;
virProcessKill;
+virProcessKillPainfully;
virProcessTranslateStatus;
virProcessWait;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7ac53ac..22fef7a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2006,13 +2006,11 @@ qemuDomainDestroyFlags(virDomainPtr dom,
* it now means the job will be released
*/
if (flags & VIR_DOMAIN_DESTROY_GRACEFUL) {
- if (qemuProcessKill(driver, vm, 0) < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("failed to kill qemu process with SIGTERM"));
+ if (qemuProcessKill(driver, vm, 0) < 0)
goto cleanup;
- }
} else {
- ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+ if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0)
+ goto cleanup;
}
/* We need to prevent monitor EOF callback from doing our work (and sending
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3cd30af..70b72af 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3877,9 +3877,7 @@ int
qemuProcessKill(struct qemud_driver *driver,
virDomainObjPtr vm, unsigned int flags)
{
- int i, ret = -1;
- const char *signame = "TERM";
- bool driver_unlocked = false;
+ int ret;
VIR_DEBUG("vm=%s pid=%d flags=%x",
vm->def->name, vm->pid, flags);
@@ -3891,78 +3889,27 @@ qemuProcessKill(struct qemud_driver *driver,
}
}
- /* This loop sends SIGTERM (or SIGKILL if flags has
- * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
- * then waits a few iterations (10 seconds) to see if it dies. If
- * the qemu process still hasn't exited, and
- * VIR_QEMU_PROCESS_KILL_FORCE is requested, a SIGKILL will then
- * be sent, and qemuProcessKill will wait up to 5 seconds more for
- * the process to exit before returning. Note that the FORCE mode
- * could result in lost data in the guest, so it should only be
- * used if the guest is hung and can't be destroyed in any other
- * manner.
- */
- for (i = 0 ; i < 75; i++) {
- int signum;
- if (i == 0) {
- if ((flags & VIR_QEMU_PROCESS_KILL_FORCE) &&
- (flags & VIR_QEMU_PROCESS_KILL_NOWAIT)) {
- signum = SIGKILL; /* kill it immediately */
- signame="KILL";
- } else {
- signum = SIGTERM; /* kindly suggest it should exit */
- }
- } else if ((i == 50) & (flags & VIR_QEMU_PROCESS_KILL_FORCE)) {
- VIR_WARN("Timed out waiting after SIG%s to process %d, "
- "sending SIGKILL", signame, vm->pid);
- signum = SIGKILL; /* kill it after a grace period */
- signame="KILL";
- } else {
- signum = 0; /* Just check for existence */
- }
-
- if (virProcessKill(vm->pid, signum) < 0) {
- if (errno != ESRCH) {
- char ebuf[1024];
- VIR_WARN("Failed to terminate process %d with SIG%s: %s",
- vm->pid, signame,
- virStrerror(errno, ebuf, sizeof(ebuf)));
- goto cleanup;
- }
- ret = 0;
- goto cleanup; /* process is dead */
- }
+ if ((flags & VIR_QEMU_PROCESS_KILL_NOWAIT)) {
+ virProcessKill(vm->pid,
+ (flags & VIR_QEMU_PROCESS_KILL_FORCE) ?
+ SIGKILL : SIGTERM);
+ return 0;
+ }
- if (i == 0 && (flags & VIR_QEMU_PROCESS_KILL_NOWAIT)) {
- ret = 0;
- goto cleanup;
- }
+ if (driver)
+ qemuDriverUnlock(driver);
- if (driver && !driver_unlocked) {
- /* THREADS.txt says we can't hold the driver lock while sleeping */
- qemuDriverUnlock(driver);
- driver_unlocked = true;
- }
+ ret = virProcessKillPainfully(vm->pid,
+ !!(flags & VIR_QEMU_PROCESS_KILL_FORCE));
- usleep(200 * 1000);
- }
- VIR_WARN("Timed out waiting after SIG%s to process %d", signame, vm->pid);
-cleanup:
- if (driver_unlocked) {
- /* We had unlocked the driver, so re-lock it. THREADS.txt says
- * we can't have the domain locked when locking the driver, so
- * we must first unlock the domain. BUT, before we can unlock
- * the domain, we need to add a ref to it in case there aren't
- * any active jobs (analysis of all callers didn't reveal such
- * a case, but there are too many to maintain certainty, so we
- * will do this as a precaution).
- */
+ if (driver) {
virObjectRef(vm);
virDomainObjUnlock(vm);
qemuDriverLock(driver);
virDomainObjLock(vm);
virObjectUnref(vm);
}
+
return ret;
}
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 958f5f7..c70aa58 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -235,3 +235,60 @@ int virProcessKill(pid_t pid, int sig)
return kill(pid, sig);
#endif
}
+
+
+/*
+ * Try to kill the process and verify it has exited
+ *
+ * Returns 0 if it was killed gracefully, 1 if it
+ * was killed forcably, -1 if it is still alive,
+ * or another error occurred.
+ */
+int
+virProcessKillPainfully(pid_t pid, bool force)
+{
+ int i, ret = -1;
+ const char *signame = "TERM";
+
+ VIR_DEBUG("vpid=%d force=%d", pid, force);
+
+ /* This loop sends SIGTERM, then waits a few iterations (10 seconds)
+ * to see if it dies. If the process still hasn't exited, and
+ * @force is requested, a SIGKILL will be sent, and this will
+ * wait upto 5 seconds more for the process to exit before
+ * returning.
+ *
+ * Note that setting @force could result in dataloss for the process.
+ */
+ for (i = 0 ; i < 75; i++) {
+ int signum;
+ if (i == 0) {
+ signum = SIGTERM; /* kindly suggest it should exit */
+ } else if ((i == 50) & force) {
+ VIR_DEBUG("Timed out waiting after SIGTERM to process %d, "
+ "sending SIGKILL", pid);
+ signum = SIGKILL; /* kill it after a grace period */
+ signame = "KILL";
+ } else {
+ signum = 0; /* Just check for existence */
+ }
+
+ if (virProcessKill(pid, signum) < 0) {
+ if (errno != ESRCH) {
+ virReportSystemError(errno,
+ _("Failed to terminate process %d with SIG%s"),
+ pid, signame);
+ goto cleanup;
+ }
+ ret = signum == SIGTERM ? 0 : 1;
+ goto cleanup; /* process is dead */
+ }
+
+ usleep(200 * 1000);
+ }
+
+ VIR_DEBUG("Timed out waiting after SIGKILL to process %d", pid);
+
+cleanup:
+ return ret;
+}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 048a73c..d537d92 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -38,5 +38,7 @@ virProcessWait(pid_t pid, int *exitstatus)
int virProcessKill(pid_t pid, int sig);
+int virProcessKillPainfully(pid_t pid, bool force);
+
#endif /* __VIR_PROCESS_H__ */
--
1.7.11.2
12 years, 1 month
