- Devel - libvirt List Archives

[libvirt] CPU topology 'sockets' handling guest vs host
by Daniel P. Berrange 07 Jun '12

07 Jun '12

On my x86_64 host I have a pair of Quad core CPUs, each in a separate NUMA node. The virsh capabilities topology data reports this: # virsh capabilities | xmllint --xpath /capabilities/host/cpu - <cpu> <arch>x86_64</arch> <model>Opteron_G3</model> <vendor>AMD</vendor> <topology sockets="1" cores="4" threads="1"/> <feature name="osvw"/> <feature name="3dnowprefetch"/> <feature name="cr8legacy"/> <feature name="extapic"/> <feature name="cmp_legacy"/> <feature name="3dnow"/> <feature name="3dnowext"/> <feature name="pdpe1gb"/> <feature name="fxsr_opt"/> <feature name="mmxext"/> <feature name="ht"/> <feature name="vme"/> </cpu> # virsh capabilities | xmllint --xpath /capabilities/host/topology - <topology> <cells num="2"> <cell id="0"> <cpus num="4"> <cpu id="0"/> <cpu id="1"/> <cpu id="2"/> <cpu id="3"/> </cpus> </cell> <cell id="1"> <cpus num="4"> <cpu id="4"/> <cpu id="5"/> <cpu id="6"/> <cpu id="7"/> </cpus> </cell> </cells> </topology> Note, it is reporting sockets=1, because sockets is the number of sockets *per* NUMA node. Now I try to figure the guest to match the host using: <cpu> <topology sockets='1' cores='4' threads='1'/> <numa> <cell cpus='0-3' memory='512000'/> <cell cpus='4-7' memory='512000'/> </numa> </cpu> And I get: error: Maximum CPUs greater than topology limit So, the XML checker is mistaking 'sockets' as the total number of sockets, rather than the per-node socket count. We need to fix this bogus check Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

3 4

[libvirt] [PATCH] Fix missing ) in 2 strings
by Cole Robinson 07 Jun '12

07 Jun '12

https://bugzilla.redhat.com/show_bug.cgi?id=801656 Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/conf/network_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6515efe..60cd888 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1200,7 +1200,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) case VIR_NETWORK_FORWARD_PASSTHROUGH: if (def->bridge) { virNetworkReportError(VIR_ERR_XML_ERROR, - _("bridge name not allowed in %s mode (network '%s'"), + _("bridge name not allowed in %s mode (network '%s')"), virNetworkForwardTypeToString(def->forwardType), def->name); goto error; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 0d6d0b6..8729f9c 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2663,7 +2663,7 @@ virNWFilterDefParse(virConnectPtr conn ATTRIBUTE_UNUSED, virNWFilterDefPtr def = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)")))) { def = virNWFilterDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } -- 1.7.7.6

2 2

[libvirt] [PATCH 1/1] Assign correct address type to spapr-vlan and spapr-vty.
by Li Zhang 07 Jun '12

07 Jun '12

For pseires guest, spapr-vlan and spapr-vty is based on spapr-vio address. According to model of network device, the address type should be assigned automatically. For serial device, serial pty device is recognized as spapr-vty device, which is also on spapr-vio. So this patch is to correct the address type of spapr-vlan and spapr-vty, and build correct command line of spapr-vty. Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com> Reviewed-by: Michael Ellerman<michaele(a)au1.ibm.com> --- src/qemu/qemu_command.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9f99dce..3555756 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -790,6 +790,9 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) /* Default values match QEMU. See spapr_(llan|vscsi|vty).c */ for (i = 0 ; i < def->nnets; i++) { + if (def->nets[i]->model && + STREQ(def->nets[i]->model, "spapr-vlan")) + def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; rc = qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, 0x1000ul); if (rc) @@ -802,16 +805,20 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) model = qemuDefaultScsiControllerModel(def); if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI && - def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - rc = qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, - 0x2000ul); - if (rc) - return rc; - } + rc = qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, + 0x2000ul); + if (rc) + return rc; } for (i = 0 ; i < def->nserials; i++) { + if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + def->serials[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY && + STREQ(def->os.arch, "ppc64") && + STREQ(def->os.machine, "pseries")) + def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; rc = qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, 0x30000000ul); if (rc) @@ -6175,10 +6182,14 @@ qemuBuildChrDeviceStr(virDomainChrDefPtr serial, virBuffer cmd = VIR_BUFFER_INITIALIZER; if (STREQ(os_arch, "ppc64") && STREQ(machine, "pseries")) { - virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", - serial->info.alias); - if (qemuBuildDeviceAddressStr(&cmd, &serial->info, qemuCaps) < 0) - goto error; + if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + serial->source.type == VIR_DOMAIN_CHR_TYPE_PTY && + serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { + virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", + serial->info.alias); + if (qemuBuildDeviceAddressStr(&cmd, &serial->info, qemuCaps) < 0) + goto error; + } } else virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s", serial->info.alias, serial->info.alias); -- 1.7.9.5

2 2

[libvirt] [PATCH] build: allow building with newer glibc-headers and -O0
by Eric Blake 07 Jun '12

07 Jun '12

glibc 2.15 (on Fedora 17) coupled with explicit disabling of optimization during development dies a painful death: In file included from /usr/include/limits.h:27:0, from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/limits.h:169, from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/syslimits.h:7, from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/limits.h:34, from util/bitmap.c:26: /usr/include/features.h:314:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp] cc1: all warnings being treated as errors Work around this by only conditionally defining _FORTIFY_SOURCE, in the case where glibc can actually use it. The trick is using AH_VERBATIM instead of AC_DEFINE. * m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Squelch _FORTIFY_SOURCE when needed to avoid glibc #warnings. --- Pushing under the build-breaker rule. m4/virt-compile-warnings.m4 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 5527bff..a91d69f 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -102,8 +102,12 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # Silence certain warnings in gnulib, and use improved glibc headers AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.]) - AC_DEFINE([_FORTIFY_SOURCE], [2], - [enable compile-time and run-time bounds-checking, and some warnings]) + AH_VERBATIM([FORTIFY_SOURCE], + [/* Enable compile-time and run-time bounds-checking, and some warnings. */ + #if __OPTIMIZE__ + # define _FORTIFY_SOURCE 2 + #endif + ]) # Extra special flags dnl -fstack-protector stuff passes gl_WARN_ADD with gcc -- 1.7.10.2

2 7

[libvirt] [PATCH] command: avoid potential deadlock on handshake
by Eric Blake 07 Jun '12

07 Jun '12

Similar to commit 858c247, the parent process should close it's copy of the end of the pipe given to the child; otherwise, if there is an extreme bug where the parent thinks the child reported an error and is waiting for the message to go along with it, but the child thinks it reported success and is waiting for the parent to acknowledge the success, we would get into deadlock. Thankfully, I don't think this deadlock is possible without at least one other bug in the code, but I did see exactly that sort of situation prior to commit da831af - if a double close bug in the parent causes the parent to read the wrong fd, it might assume the child failed, even though the child really sent success if only the parent had read from the correct location. * src/util/command.c (virCommandHandshakeWait): Close unused fds sooner. --- src/util/command.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util/command.c b/src/util/command.c index 62ea50b..62848cd 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -2507,6 +2507,11 @@ int virCommandHandshakeWait(virCommandPtr cmd) VIR_FORCE_CLOSE(cmd->handshakeWait[0]); return -1; } + /* Close the handshakeNotify fd before trying to read anything + * further on the handshakeWait pipe; so that a child waiting + * on our acknowledgment will die rather than deadlock. */ + VIR_FORCE_CLOSE(cmd->handshakeNotify[1]); + if ((len = saferead(cmd->handshakeWait[0], msg, 1024)) < 0) { VIR_FORCE_CLOSE(cmd->handshakeWait[0]); VIR_FREE(msg); -- 1.7.10.2

2 2

[libvirt] RFC: TODO list for a 1.0 release
by Daniel P. Berrange 07 Jun '12

07 Jun '12

We have mentioned a 1.0 release in passing a few times recently but we have never really set out a clear list of goals for such a notable release. This thread is an attempt to clarify such goals. To avoid making the 1.0 target too hard, we should aim for as *little* as possible on our TODO list. I think the priority here should be on public API level things, or core libvirt infrastructure, and not random impl details of specific hypervisors. In particular I think we should focus on things that make libvirt better to develop app against. IMHO we should have the following things in the 1.0 release - List object APIs which directly return the object instance https://bugzilla.redhat.com/show_bug.cgi?id=636096 * virConnectListAllDomains * virConnectListAllInterfaces * virConnectListAllNetworks * virConnectListAllNWFIlters * virConnectListAllNodeDevices * virConnectListAllSecrets * virConnectListAllStoragePools * virDomainListAllSnapshots * virStoragePoolListAllVolumes NB: with support across LXC, UML, Xen, LibXL, QEMU & ESX - Lifecycle events for all top level objects https://bugzilla.redhat.com/show_bug.cgi?id=636027 * virConnectInterfaceEventRegisterAny * virConnectNetworkEventRegisterAny * virConnectNWFilterEventRegisterAny * virConnectNodeDeviceEventRegisterAny * virConnectSecretEventRegisterAny * virConnectStoragePoolEventRegisterAny - Fine grained access control https://bugzilla.redhat.com/show_bug.cgi?id=636148 * Access control infrastructure * PolicyKit driver impl * Simple RBAC driver impl * SELinux driver impl (probably not needed for 1.0) Do you have suggestions for anything else that you think is very important for libvirt ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

6 8

[libvirt] [PATCH] util: Fix deadlock in virLogReset
by Jiri Denemark 07 Jun '12

07 Jun '12

When libvirtd forks off a new child, the child then calls virLogReset(), which ends up closing file descriptors used as log outputs. However, we recently started logging closed file descriptors, which means we need to lock logging mutex which was already logged by virLogReset(). We don't really want to log anything when we are in the process of closing log outputs. --- src/util/logging.c | 5 +++-- src/util/virfile.c | 28 ++++++++++++++++------------ src/util/virfile.h | 22 ++++++++++++++++++---- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/util/logging.c b/src/util/logging.c index 23778d3..cf62184 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -864,10 +864,11 @@ static int virLogOutputToFd(const char *category ATTRIBUTE_UNUSED, return ret; } -static void virLogCloseFd(void *data) { +static void virLogCloseFd(void *data) +{ int fd = (intptr_t) data; - VIR_FORCE_CLOSE(fd); + VIR_LOG_CLOSE(fd); } static int virLogAddOutputToStderr(int priority) { diff --git a/src/util/virfile.c b/src/util/virfile.c index a0000d0..6c69217 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -43,7 +43,7 @@ __FUNCTION__, __LINE__, __VA_ARGS__) -int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF) +int virFileClose(int *fdptr, virFileCloseFlags flags) { int saved_errno = 0; int rc = 0; @@ -51,24 +51,28 @@ int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF) if (*fdptr < 0) return 0; - if (preserve_errno) + if (flags & VIR_FILE_CLOSE_PRESERVE_ERRNO) saved_errno = errno; rc = close(*fdptr); - if (rc < 0) { - if (errno == EBADF) { - if (!ignore_EBADF) - VIR_WARN("Tried to close invalid fd %d", *fdptr); + + if (!(flags & VIR_FILE_CLOSE_DONT_LOG)) { + if (rc < 0) { + if (errno == EBADF) { + if (!(flags & VIR_FILE_CLOSE_IGNORE_EBADF)) + VIR_WARN("Tried to close invalid fd %d", *fdptr); + } else { + char ebuf[1024] ATTRIBUTE_UNUSED; + VIR_DEBUG("Failed to close fd %d: %s", + *fdptr, virStrerror(errno, ebuf, sizeof(ebuf))); + } } else { - char ebuf[1024] ATTRIBUTE_UNUSED; - VIR_DEBUG("Failed to close fd %d: %s", - *fdptr, virStrerror(errno, ebuf, sizeof(ebuf))); + VIR_DEBUG("Closed fd %d", *fdptr); } - } else { - VIR_DEBUG("Closed fd %d", *fdptr); } *fdptr = -1; - if (preserve_errno) + + if (flags & VIR_FILE_CLOSE_PRESERVE_ERRNO) errno = saved_errno; return rc; diff --git a/src/util/virfile.h b/src/util/virfile.h index c033248..6882a73 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -31,16 +31,21 @@ # include "internal.h" # include "ignore-value.h" +typedef enum virFileCloseFlags { + VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0, + VIR_FILE_CLOSE_IGNORE_EBADF = 1 << 1, + VIR_FILE_CLOSE_DONT_LOG = 1 << 2, +} virFileCloseFlags; /* Don't call these directly - use the macros below */ -int virFileClose(int *fdptr, bool preserve_errno, bool ignore_EBADF) +int virFileClose(int *fdptr, virFileCloseFlags flags) ATTRIBUTE_RETURN_CHECK; int virFileFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK; /* For use on normal paths; caller must check return value, and failure sets errno per close. */ -# define VIR_CLOSE(FD) virFileClose(&(FD), false, false) +# define VIR_CLOSE(FD) virFileClose(&(FD), 0) # define VIR_FCLOSE(FILE) virFileFclose(&(FILE), false) /* Wrapper around fdopen that consumes fd on success. */ @@ -48,12 +53,21 @@ FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK; /* For use on cleanup paths; errno is unaffected by close, and no return value to worry about. */ -# define VIR_FORCE_CLOSE(FD) ignore_value(virFileClose(&(FD), true, false)) +# define VIR_FORCE_CLOSE(FD) \ + ignore_value(virFileClose(&(FD), VIR_FILE_CLOSE_PRESERVE_ERRNO)) # define VIR_FORCE_FCLOSE(FILE) ignore_value(virFileFclose(&(FILE), true)) /* Similar VIR_FORCE_CLOSE() but ignores EBADF errors since they are expected * during mass close after fork(). */ -# define VIR_MASS_CLOSE(FD) ignore_value(virFileClose(&(FD), true, true)) +# define VIR_MASS_CLOSE(FD) \ + ignore_value(virFileClose(&(FD), \ + VIR_FILE_CLOSE_PRESERVE_ERRNO | \ + VIR_FILE_CLOSE_IGNORE_EBADF)) + +# define VIR_LOG_CLOSE(FD) \ + ignore_value(virFileClose(&(FD), \ + VIR_FILE_CLOSE_PRESERVE_ERRNO | \ + VIR_FILE_CLOSE_DONT_LOG)) /* Opaque type for managing a wrapper around a fd. */ struct _virFileWrapperFd; -- 1.7.10.2

2 1

[libvirt] Looking for libvirt for RHEL 6.1 to provide Memory related metrics.
by Roy, Kallol 07 Jun '12

07 Jun '12

Hello All, Can some one let me know if we have the latest libvirt version for RHEL 6.1 which would provided me Memory related counters/metrics. The latest libvirt which I found was on Fedora. Any pointer or expected dates of libvirt release on RHEL would be highly appreciated. Thanks and Regards, _____________________________________________________________________ Kallol Roy "Be kinder than necessary, for everyone you meet is fighting some kind of battle."

3 4

[libvirt] [PATCH] qemu: better detection of crashed domains
by Martin Kletzander 06 Jun '12

06 Jun '12

When libvirtd is started and there is an unusable/not-connectable leftover from earlier started machine, it's more reasonable to say that the machine "crashed" if we know it was started with "-no-shutdown". This patch fixes that and also changes the other result (when machine was started without "-no-shutdown") to "unknown", because the previous "failed" reason means (according to include/libvirt/libvirt.h.in:174), that the machine failed to start. --- src/qemu/qemu_process.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2adf570..604c31b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3141,7 +3141,17 @@ error: * to remove danger of it ending up running twice if * user tries to start it again later */ - qemuProcessStop(driver, obj, 0, VIR_DOMAIN_SHUTOFF_FAILED); + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) + /* If we couldn't get the monitor and qemu supports + * no-shutdown, we can safely say that the domain + * crashed ... */ + state = VIR_DOMAIN_SHUTOFF_CRASHED; + else + /* ... but if it doesn't we can't say what the state + * really is and FAILED means "failed to start" */ + state = VIR_DOMAIN_SHUTOFF_UNKNOWN; + + qemuProcessStop(driver, obj, 0, state); if (!obj->persistent) qemuDomainRemoveInactive(driver, obj); else -- 1.7.8.6

2 2

[libvirt] [PATCH] build: fix build without i18n
by Eric Blake 06 Jun '12

06 Jun '12

If you compile without NLS support, where _() is a no-op macro, then we end up passing a string literal to a char*, provoking: In file included from virsh.c:3639:0: virsh-edit.c: In function ‘cmdSaveImageEdit’: virsh-edit.c:97:13: error: assignment discards ‘const’ qualifier from pointer target type [-Werror] virsh-edit.c:106:13: error: assignment discards ‘const’ qualifier from pointer target type [-Werror] * tools/virsh-edit.c: Be const-safe. --- Pushing under the build-breaker rule. tools/virsh-edit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c index aa59f25..27140e2 100644 --- a/tools/virsh-edit.c +++ b/tools/virsh-edit.c @@ -55,7 +55,7 @@ do { char *doc = NULL; char *doc_edited = NULL; char *doc_reread = NULL; - char *msg = NULL; + const char *msg = NULL; /* Get the XML configuration of the object. */ doc = (EDIT_GET_XML); -- 1.7.10.2

1 0