- Devel - libvirt List Archives

[libvirt] cygwin: #include netinet/inet.h -- needed only on cygwin
by Stefan Berger 18 Aug '10

18 Aug '10

When doing './autogen.sh --with-sasl --with-libvirtd=no' on cygwin, I needed to #include netinet/in.h for a successful build. Now authentication using SASL works. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> diff --git a/src/remote/qemu_protocol.h b/src/remote/qemu_protocol.h index b822187..8d806fc 100644 --- a/src/remote/qemu_protocol.h +++ b/src/remote/qemu_protocol.h @@ -6,6 +6,9 @@ #ifndef _RP_QEMU_H_RPCGEN #define _RP_QEMU_H_RPCGEN +#ifdef __CYGWIN__ +# include <netinet/in.h> +#endif #include <rpc/rpc.h> diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index afe9287..113818d 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -6,6 +6,9 @@ #ifndef _RP_H_RPCGEN #define _RP_H_RPCGEN +#ifdef __CYGWIN__ +# include <netinet/in.h> +#endif #include <rpc/rpc.h>

4 6

Re: [libvirt] how to print size_t in LGPLv2+ program
by Eric Blake 18 Aug '10

18 Aug '10

On 08/17/2010 03:47 PM, Paul Eggert wrote: > On 08/17/10 23:17, Eric Blake wrote: > >> libvirt is currently LGPLv2+ > > Can this be changed to LPGLv3+? That'd solve the problem. True, but it would have knock-on effects on all sorts of other clients of libvirt. I'm re-adding the libvirt list for thoughts from anyone else. > >> Is it worth relaxing the license on the *printf-posix family of modules >> to LGPLv2+ from their current LGPLv3+, or is this too big of a request? > > I dunno, at some point we should be encouraging people to switch > to LGPLv3+. It makes sense for the FSF to request that GNU projects use better licenses. Unfortunately, libvirt is not a GNU project; so it is more a question as to how Red Hat wants to license it, rather than what the FSF prefers. At any rate, I think the ultimate decision on how libvirt is licensed is rather political in nature, and will involve quite a bit of discussion among more than just the software developers contributing to libvirt. But at least libvirt is LGPLv2+; unlike some projects (like git) that are explicitly v2-only and can't do the upgrade. Meanwhile, I tend to agree that relaxing printf-posix to LGPLv2+ is a rather hefty sledge-hammer, and probably not appropriate for gnulib. > >> since using umaxtostr penalizes 32-bit machines for converting to the >> 64-bit intermediary, maybe it's worth adding a size_t variant? > > That would be fine. The *tostr routines were mainly written > for two reasons. First, for platforms that can't/couldn't portably > and reliably output wide integers. Second, speed. > >> What a shame that POSIX omitted an <inttypes.h> PRIu* for size_t. > > We could add one in gnulib, no? In a quick google search, I found this use of PRIdS (which is geared more for ssize_t, but the analog PRIuS would apply to size_t): http://bytes.com/topic/c/answers/506972-64-bit-portability-size_t-printf-fo… However, S is awfully short; I'd prefer something a bit longer like PRIuSIZE. Is there any other project already using gnulib that has invented a PRIuXXX name for size_t? It would also be nice if we had some prior art from glibc or even one of the BSD systems to copy a commonly-used name. Meanwhile, the idea is slightly more appealing than relicensing lots of gnulib or using the inttostr code; but it still requires auditing code and forbidding "%zu" in favor of "%"PRIuSIZE (or whatever other name we settle on). Also, it seems like we might want to create a new intttypes-plus (or inttypes-gnu) module, and leave the existing inttypes module as providing just the namespace guaranteed by glibc, while requiring the use of the new module to pollute the namespace with these new (but useful) macros. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

4 3

[libvirt] [PATCH] esx: Fix memory leak when looking up an non-existing domain by name
by Matthias Bolte 17 Aug '10

17 Aug '10

In case an optional object cannot be found the lookup function is left early and the cleanup code is not executed. Add a success label and goto instead of an early return. This pattern occurs in some other functions too. --- src/esx/esx_vi.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index c6b2b63..b064b11 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2254,7 +2254,7 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid, if (managedObjectReference == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - return 0; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_DOMAIN, _("Could not find domain with UUID '%s'"), @@ -2270,6 +2270,7 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid, goto cleanup; } + success: result = 0; cleanup: @@ -2327,7 +2328,7 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, if (*virtualMachine == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - return 0; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_DOMAIN, _("Could not find domain with name '%s'"), name); @@ -2335,6 +2336,7 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, } } + success: result = 0; cleanup: @@ -2890,7 +2892,7 @@ esxVI_LookupCurrentSnapshotTree if (currentSnapshot == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - return 0; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", _("Domain has no current snapshot")); @@ -2911,6 +2913,7 @@ esxVI_LookupCurrentSnapshotTree goto cleanup; } + success: result = 0; cleanup: @@ -3050,9 +3053,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, /* Interpret search result */ if (searchResults->file == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - result = 0; - - goto cleanup; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_STORAGE_VOL, _("No storage volume with key or path '%s'"), @@ -3064,6 +3065,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, *fileInfo = searchResults->file; searchResults->file = NULL; + success: result = 0; cleanup: -- 1.7.0.4

2 4

[libvirt] [PATCH 0/4]
by Daniel P. Berrange 17 Aug '10

17 Aug '10

For https://bugzilla.redhat.com/show_bug.cgi?id=620847 We have had sporadic reports of # virsh capabilities error: failed to get capabilities error: server closed connection: This normally means that libvirtd has crashed, closing the connection but in this case libvirtd has always remained running. It turns out that the capabilities XML was too large for the remote RPC message size. This caused XDR serialization to fail. This caused libvirtd to close the client connection immediately. The cause of the large XML was node handling an edge case in libnuma where it returns a CPU mask of all-1s to indicate a non-existant node. Machines that exhibit this problem will show this as a symptom in the logs # grep NUMA /var/log/messages Aug 16 10:30:34 sgi-xe270-01 libvirtd: 10:30:34.933: warning : nodeCapsInitNUMA:388 : NUMA topology for cell 1 of 2 not available, ignoring And have sparse NUMA topology (ie empty nodes) This series does many things: - Adds explicit warnings in places where XDR serialization fails, so we see an indication of problem in /var/log/messages - Try to send a real remote_error back to client, instead of closing its connection - Add logging of capabilities XML in libvirt.c so we can identify the too large doc in libvirtd - Add fix to cope with all-1s node mask This may also fix some other unexplained bug reports we've had with 'server closed connection' messages, or at least make it possible to diagnose them Daniel

3 6

[libvirt] [PATCH] Add support for -enable-kqemu flag
by Daniel P. Berrange 17 Aug '10

17 Aug '10

Previously QEMU enabled KQEMU by default and had -no-kqemu. 0.11.x switched to requiring -enable-kqemu. 0.12.x dropped kqemu entirely. This patch adds support for -enable-kqemu so 0.11.x works. It replaces a huge set of if() with a switch() to make the code a bit more readable. This patch was inspired by the one John Lumby send to the list a while ago, but the way I've implemented it is more in line with the logic we have for KVM. http://www.redhat.com/archives/libvir-list/2010-July/msg00368.html * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Support -enable-kqemu --- src/qemu/qemu_conf.c | 84 +++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_conf.h | 1 + 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fb85220..0ddf128 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1156,6 +1156,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help, if (strstr(help, "-no-kqemu")) flags |= QEMUD_CMD_FLAG_KQEMU; + if (strstr(help, "-enable-kqemu")) + flags |= QEMUD_CMD_FLAG_ENABLE_KQEMU; if (strstr(help, "-no-kvm")) flags |= QEMUD_CMD_FLAG_KVM; if (strstr(help, "-enable-kvm")) @@ -3662,6 +3664,7 @@ int qemudBuildCommandLine(virConnectPtr conn, char boot[VIR_DOMAIN_BOOT_LAST]; struct utsname ut; int disableKQEMU = 0; + int enableKQEMU = 0; int disableKVM = 0; int enableKVM = 0; int qargc = 0, qarga = 0; @@ -3711,38 +3714,59 @@ int qemudBuildCommandLine(virConnectPtr conn, emulator = def->emulator; - /* Need to explicitly disable KQEMU if - * 1. Guest domain is 'qemu' - * 2. The qemu binary has the -no-kqemu flag - */ - if ((qemuCmdFlags & QEMUD_CMD_FLAG_KQEMU) && - def->virtType == VIR_DOMAIN_VIRT_QEMU) - disableKQEMU = 1; - - /* Need to explicitly disable KVM if - * 1. Guest domain is 'qemu' - * 2. The qemu binary has the -no-kvm flag + /* + * do not use boot=on for drives when not using KVM since this + * is not supported at all in upstream QEmu. */ if ((qemuCmdFlags & QEMUD_CMD_FLAG_KVM) && - def->virtType == VIR_DOMAIN_VIRT_QEMU) { - disableKVM = 1; + (def->virtType == VIR_DOMAIN_VIRT_QEMU) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_BOOT)) + qemuCmdFlags -= QEMUD_CMD_FLAG_DRIVE_BOOT; + + switch (def->virtType) { + case VIR_DOMAIN_VIRT_QEMU: + if (qemuCmdFlags & QEMUD_CMD_FLAG_KQEMU) + disableKQEMU = 1; + if (qemuCmdFlags & QEMUD_CMD_FLAG_KVM) + disableKVM = 1; + break; - /* - * do not use boot=on for drives when not using KVM since this - * is not supported at all in upstream QEmu. - */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_BOOT) - qemuCmdFlags -= QEMUD_CMD_FLAG_DRIVE_BOOT; - } + case VIR_DOMAIN_VIRT_KQEMU: + if (qemuCmdFlags & QEMUD_CMD_FLAG_KVM) + disableKVM = 1; - /* Should explicitly enable KVM if - * 1. Guest domain is 'kvm' - * 2. The qemu binary has the -enable-kvm flag - * NOTE: user must be responsible for loading the kvm modules - */ - if ((qemuCmdFlags & QEMUD_CMD_FLAG_ENABLE_KVM) && - def->virtType == VIR_DOMAIN_VIRT_KVM) - enableKVM = 1; + if (qemuCmdFlags & QEMUD_CMD_FLAG_ENABLE_KQEMU) { + enableKQEMU = 1; + } else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_KQEMU)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support kqemu"), + emulator); + } + break; + + case VIR_DOMAIN_VIRT_KVM: + if (qemuCmdFlags & QEMUD_CMD_FLAG_KQEMU) + disableKQEMU = 1; + + if (qemuCmdFlags & QEMUD_CMD_FLAG_ENABLE_KVM) { + enableKVM = 1; + } else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_KVM)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support kvm"), + emulator); + } + break; + + case VIR_DOMAIN_VIRT_XEN: + /* XXX better check for xenner */ + break; + + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support %s"), + emulator, virDomainVirtTypeToString(def->virtType)); + break; + } #define ADD_ARG_SPACE \ do { \ @@ -3857,6 +3881,10 @@ int qemudBuildCommandLine(virConnectPtr conn, if (disableKQEMU) ADD_ARG_LIT("-no-kqemu"); + else if (enableKQEMU) { + ADD_ARG_LIT("-enable-kqemu"); + ADD_ARG_LIT("-kernel-kqemu"); + } if (disableKVM) ADD_ARG_LIT("-no-kvm"); if (enableKVM) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 1aa9d2e..4f9c714 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -92,6 +92,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_PCI_CONFIGFD = (1LL << 36), /* pci-assign.configfd */ QEMUD_CMD_FLAG_NODEFCONFIG = (1LL << 37), /* -nodefconfig */ QEMUD_CMD_FLAG_BOOT_MENU = (1LL << 38), /* -boot menu=on support */ + QEMUD_CMD_FLAG_ENABLE_KQEMU = (1LL << 39), /* Is the -enable-kqemu flag available to "enable KQEMU full virtualization support" */ }; /* Main driver state */ -- 1.7.2.1

2 1

[libvirt] [PATCH] docs: mention domain <clock> improvements
by Eric Blake 17 Aug '10

17 Aug '10

Add documentation for features added a while ago. * docs/formatdomain.html.in (Time keeping): Update documentation of <clock> element to match 0.8.0 addition. --- I've had this sitting on my tree for a while; any suggestions for improvements, or should I at least check it in as-is so that we have a start on this documentation? docs/formatdomain.html.in | 76 ++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 71 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5269cc5..f9a153b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -411,13 +411,18 @@ <pre> ... - <clock offset="localtime"/> + <clock offset="localtime"> + <timer name="rtc" tickpolicy="catchup" track="guest"> + <catchup threshold=123 slew=120 limit=10000/> + </timer> + <timer name="pit" tickpolicy="none"/> + </clock> ...</pre> <dl> <dt><code>clock</code></dt> <dd> - The <code>offset</code> attribute takes three possible + The <code>offset</code> attribute takes four possible values, allowing fine grained control over how the guest clock is synchronized to the host. NB, not all hypervisors support all modes. @@ -435,6 +440,7 @@ <dd> The guest clock will be synchronized to the requested timezone using the <code>timezone</code> attribute. + Since 0.7.7 </dd> <dt><code>variable</code></dt> <dd> @@ -444,14 +450,74 @@ The guest is free to adjust the RTC over time an expect that it will be honoured at next reboot. This is in contrast to 'utc' mode, where the RTC adjustments are - lost at each reboot. + lost at each reboot. Since 0.7.7 </dd> </dl> - NB, at time of writing, only QEMU supports the variable - clock mode, or custom timezones. + A <code>clock</code> may have zero or more + <code>timer</code>sub-elements. Since + 0.8.0 </dd> + <dt><code>timer</code></dt> + <dd> + + Each timer element requires a <code>name</code> attribute, + and has other optional attributes that depend on + the <code>name</code> specified. Various hypervisors + support different combinations of attributes. + + <dl> + <dt><code>name</code></dt> + <dd> + The <code>name</code> attribute selects which timer is + being modified, and can be one of "platform", "pit", + "rtc", "hpet", or "tsc". + </dd> + <dt><code>track</code></dt> + <dd> + The <code>track</code> attribute specifies what the timer + tracks, and can be "boot", "guest", or "wall". + Only valid for <code>name="rtc"</code> + or <code>name="platform"</code>. + </dd> + <dt><code>tickpolicy</code></dt> + <dd> + The <code>tickpolicy</code> attribute determines how + missed ticks in the guest are handled, and can be "delay", + "catchup", "merge", or "discard". If the policy is + "catchup", there can be further details in + the <code>catchup</code> sub-element. + <dl> + <dt><code>catchup</code></dt> + <dd> + The <code>catchup</code> element has three optional + attributes, each a positive integer. The attributes + are <code>threshold</code>, <code>slew</code>, + and <code>limit</code>. + </dd> + </dl> + </dd> + <dt><code>frequency</code></dt> + <dd> + The <code>frequency</code> attribute is an unsigned + integer specifying the frequency at + which <code>name="tsc"</code> runs. + </dd> + <dt><code>mode</code></dt> + <dd> + The <code>mode</code> attribute controls how + the <code>name="tsc"</code> timer is managed, and can be + "auto", "native", "emulate", "paravirt", or "smpsafe". + Other timers are always emulated. + </dd> + <dt><code>present</code></dt> + <dd> + The <code>present</code> attribute can be "yes" or "no" to + specify whether a particular timer is available to the guest. + </dd> + </dl> + </dd> </dl> <h3><a name="elementsDevices">Devices</a></h3> -- 1.7.2.1

2 2

[libvirt] [PATCH 0/4] Fix problem with capabilities XML generation
by Daniel P. Berrange 17 Aug '10

17 Aug '10

We have had sporadic reports of # virsh capabilities error: failed to get capabilities error: server closed connection: This normally means that libvirtd has crashed, closing the connection but in this case libvirtd has always remained running. It turns out that the capabilities XML was too large for the remote RPC message size. This caused XDR serialization to fail. This caused libvirtd to close the client connection immediately. The cause of the large XML was node handling an edge case in libnuma where it returns a CPU mask of all-1s to indicate a non-existant node. This series does many things: - Adds explicit warnings in places where XDR serialization fails, so we see an indication of problem in /var/log/messages - Try to send a real remote_error back to client, instead of closing its connection - Add logging of capabilities XML in libvirt.c so we can identify the too large doc in libvirtd - Add fix to cope with all-1s node mask

3 13

[libvirt] [PATCH] cygwin: build fix
by Stefan Berger 17 Aug '10

17 Aug '10

Fixing a problem in the build on cygwin due to missing #define's. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/remote/remote_protocol.c | 6 ++++++ src/remote/remote_protocol.h | 6 ++++++ src/remote/remote_protocol.x | 6 ++++++ 3 files changed, 18 insertions(+) Index: libvirt-acl/src/remote/remote_protocol.h =================================================================== --- libvirt-acl.orig/src/remote/remote_protocol.h +++ libvirt-acl/src/remote/remote_protocol.h @@ -24,6 +24,12 @@ extern "C" { #ifndef IXDR_GET_INT32 # define IXDR_GET_INT32 IXDR_GET_LONG #endif +#ifndef IXDR_PUT_U_INT32 +# define IXDR_PUT_U_INT32 IXDR_PUT_U_LONG +#endif +#ifndef IXDR_GET_U_INT32 +# define IXDR_GET_U_INT32 IXDR_GET_U_LONG +#endif #define REMOTE_MESSAGE_MAX 262144 #define REMOTE_MESSAGE_HEADER_MAX 24 #define REMOTE_MESSAGE_PAYLOAD_MAX 262120 Index: libvirt-acl/src/remote/remote_protocol.x =================================================================== --- libvirt-acl.orig/src/remote/remote_protocol.x +++ libvirt-acl/src/remote/remote_protocol.x @@ -51,6 +51,12 @@ %#ifndef IXDR_GET_INT32 %# define IXDR_GET_INT32 IXDR_GET_LONG %#endif +%#ifndef IXDR_PUT_U_INT32 +%# define IXDR_PUT_U_INT32 IXDR_PUT_U_LONG +%#endif +%#ifndef IXDR_GET_U_INT32 +%# define IXDR_GET_U_INT32 IXDR_GET_U_LONG +%#endif /*----- Data types. -----*/ Index: libvirt-acl/src/remote/remote_protocol.c =================================================================== --- libvirt-acl.orig/src/remote/remote_protocol.c +++ libvirt-acl/src/remote/remote_protocol.c @@ -16,6 +16,12 @@ #ifndef IXDR_GET_INT32 # define IXDR_GET_INT32 IXDR_GET_LONG #endif +#ifndef IXDR_PUT_U_INT32 +# define IXDR_PUT_U_INT32 IXDR_PUT_U_LONG +#endif +#ifndef IXDR_GET_U_INT32 +# define IXDR_GET_U_INT32 IXDR_GET_U_LONG +#endif bool_t xdr_remote_nonnull_string (XDR *xdrs, remote_nonnull_string *objp)

2 2

[libvirt] Potential gotcha in virsh migration with --persistent?
by Justin Clift 17 Aug '10

17 Aug '10

Hi all, Just noticed that when using the virsh --persistent option for migration, that the "autostart" attribute isn't mirrored across to the destination host. This seems like a "gotcha", in that a person migrating an autostarted guest to another host with --persistent will be expecting the migration to persist _all_ of the (important at least) guest settings. Without mirroring this attribute, the person doing the migration needs to manually follow up the migration by subsequently connecting to the destination host, then turning on autostart attribute for the domain there. Guessing it's not a design decision, and is just something we should fix? Regards and best wishes, Justin Clift

2 2

[libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
by Soren Hansen 16 Aug '10

16 Aug '10

The current check for the size of the response from the uml monitor is problematic for a couple of reasons: First of all, for me, the call to recvfrom returns 0, even though the buffer actually is populated with the response from the monitor. Second, it seems to me that (assuming recvfrom actually returned the number of bytes read) it should be compared against the size of the response header plus the actual data length rather than the max size of the datagram. At any rate, the only way I can get anything useful to happen here is to completely remove the check, so this patch does just that. Signed-off-by: Soren Hansen <soren(a)linux2go.dk> --- src/uml/uml_driver.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 04493ba..9cf669f 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -737,10 +737,6 @@ static int umlMonitorCommand(const struct uml_driver *driver, virReportSystemError(errno, _("cannot read reply %s"), cmd); goto error; } - if (nbytes < sizeof res) { - virReportSystemError(0, _("incomplete reply %s"), cmd); - goto error; - } if (sizeof res.data < res.length) { virReportSystemError(0, _("invalid length in reply %s"), cmd); goto error; -- 1.7.0.4

4 12