- Devel - libvirt List Archives

[libvirt] [PATCH v2 0/9] Introduce virDomainGetState API
by Jiri Denemark 16 May '11

16 May '11

This new API solves several problems: - calling virDomainGetInfo for just getting domain status is an overkill since it may result in sending requests to guest OS - since virDomainGetInfo can hang when guest OS is not responding and it is used by virsh list, listing domains can hang - virDomainGetState provides additional info about what action led to domain's current state, which can be used instead of listening to domain events Version 2: - rebased to current HEAD - unsigned int flags parameter - updated version info in public.syms - noGetState renamed as useGetInfo in virsh.c - simplified implementation in esx driver per Matthias' suggestion - call internal xen drivers directly instead of going through xenUnifiedDriver - fixed || vs && typo in domain_conf.c - new patch 9/9: qemu: Update domain state when reconnecting monitor Jiri Denemark (9): virDomainGetState public API Internal driver API for virDomainGetState virDomainGetState public API implementation Wire protocol format and dispatcher for virDomainGetState virsh: Prefer virDomainGetState over virDomainGetInfo remote: Implement virDomainGetState Implement basic virDomainGetState in all drivers Implement domain state reason qemu: Update domain state when reconnecting monitor daemon/remote.c | 33 +++++++ daemon/remote_generator.pl | 6 +- include/libvirt/libvirt.h.in | 56 +++++++++++ python/generator.py | 1 + python/libvirt-override-api.xml | 6 + python/libvirt-override.c | 30 ++++++ src/conf/domain_conf.c | 163 +++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 26 +++++- src/driver.h | 6 + src/esx/esx_driver.c | 44 +++++++++ src/libvirt.c | 51 ++++++++++ src/libvirt_private.syms | 4 + src/libvirt_public.syms | 5 + src/libxl/libxl_driver.c | 80 ++++++++++++---- src/lxc/lxc_driver.c | 81 ++++++++++++---- src/openvz/openvz_conf.c | 16 ++- src/openvz/openvz_driver.c | 57 +++++++++-- src/phyp/phyp_driver.c | 16 +++ src/qemu/qemu_driver.c | 95 ++++++++++++++----- src/qemu/qemu_migration.c | 24 +++-- src/qemu/qemu_monitor.c | 20 ++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 42 +++++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 30 ++++++ src/qemu/qemu_monitor_text.h | 1 + src/qemu/qemu_process.c | 99 +++++++++++++++----- src/qemu/qemu_process.h | 12 ++- src/remote/remote_driver.c | 34 +++++++ src/remote/remote_protocol.x | 13 +++- src/remote_protocol-structs | 8 ++ src/test/test_driver.c | 107 +++++++++++++++------- src/uml/uml_driver.c | 58 ++++++++++-- src/vbox/vbox_tmpl.c | 60 ++++++++++++ src/vmware/vmware_conf.c | 3 +- src/vmware/vmware_driver.c | 60 ++++++++++--- src/xen/xen_driver.c | 42 +++++++++ src/xen/xen_hypervisor.c | 36 +++++++ src/xen/xen_hypervisor.h | 5 + src/xen/xend_internal.c | 99 +++++++++++++++----- src/xen/xend_internal.h | 4 + src/xen/xm_internal.c | 20 ++++ src/xen/xm_internal.h | 4 + src/xen/xs_internal.c | 34 +++++++ src/xen/xs_internal.h | 4 + src/xenapi/xenapi_driver.c | 49 ++++++++++ tools/virsh.c | 195 ++++++++++++++++++++++++++++++++------- 47 files changed, 1603 insertions(+), 238 deletions(-) -- 1.7.5.rc3

3 20

[libvirt] [PATCH v4 0/8] Add support for taking screenshots of domain console
by Michal Privoznik 16 May '11

16 May '11

This series adds support for taking screenshots of a running domain console. The iohelper was added a new argument - delete file after transfer. This is needed, because the screenshot is written to file and asynchronously transferred to client. New API is accessible via virsh screenshot <domain> <path> <screnID>; For now, we just save the file in format as returned by hypervisor: PPM for Qemu, PNG for VirtualBox. diff to v3: - added screen ID argument diff to v2: - rebase Michal Privoznik (8): screenshot: Defining the public API screenshot: Defining the internal API screenshot: Implementing the public API screenshot: Implementing the remote protocol screenshot: Expose the new API in virsh virFDStream: Add option for delete file after it's opening qemu: Implement the driver methods vbox: Implement the driver methods daemon/remote.c | 57 ++++++++++++++++++ include/libvirt/libvirt.h.in | 8 +++ src/driver.h | 8 ++- src/esx/esx_driver.c | 1 + src/fdstream.c | 29 +++++++-- src/fdstream.h | 6 +- src/libvirt.c | 60 +++++++++++++++++++ src/libvirt_public.syms | 1 + src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 4 +- src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 20 ++++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 23 +++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_monitor_text.c | 31 ++++++++++ src/qemu/qemu_monitor_text.h | 3 + src/remote/remote_driver.c | 41 +++++++++++++ src/remote/remote_protocol.x | 13 ++++- src/remote_protocol-structs | 8 +++ src/storage/storage_driver.c | 4 +- src/test/test_driver.c | 1 + src/uml/uml_driver.c | 4 +- src/util/iohelper.c | 12 +++- src/vbox/vbox_tmpl.c | 134 ++++++++++++++++++++++++++++++++++++++++++ src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 4 +- src/xen/xen_driver.h | 1 + src/xen/xen_hypervisor.c | 1 + src/xen/xen_inotify.c | 1 + src/xen/xend_internal.c | 1 + src/xen/xm_internal.c | 1 + src/xen/xs_internal.c | 1 + src/xenapi/xenapi_driver.c | 1 + tools/virsh.c | 97 ++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++ 38 files changed, 667 insertions(+), 19 deletions(-) -- 1.7.5.rc3

4 26

[libvirt] [PATCH] domain: Require <init> for container guests
by Cole Robinson 16 May '11

16 May '11

Use capabilities to allow a driver to register a default <init> if none is specified in the XML. Openvz was already open-coding this to be /sbin/init LXC currently falls over if no init is specified, so an explicit error is an improvement IMO. (Side note: I don't think we can set a default value for LXC. If we use /sbin/init but the user doesn't specify a separate root FS for their guest, the container will rerun the host's init which can be traumatic :). For virt-install I'm thinking of defaulting to /sbin/init if a root FS has been specified, otherwise require the user to manually specify <init>) Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/conf/capabilities.h | 1 + src/conf/domain_conf.c | 12 ++++++++++++ src/openvz/openvz_conf.c | 4 +++- src/openvz/openvz_driver.c | 14 -------------- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 96bf0a2..e2fa1d6 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -150,6 +150,7 @@ struct _virCaps { int (*privateDataXMLFormat)(virBufferPtr, void *); int (*privateDataXMLParse)(xmlXPathContextPtr, void *); bool hasWideScsiBus; + const char *defaultInitPath; virDomainXMLNamespace ns; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9ab9a5a..f7e4959 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5708,6 +5708,18 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (STREQ(def->os.type, "exe")) { def->os.init = virXPathString("string(./os/init[1])", ctxt); + if (!def->os.init) { + if (caps->defaultInitPath) { + def->os.init = strdup(caps->defaultInitPath); + if (!def->os.init) { + goto no_memory; + } + } else { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + goto error; + } + } } if (STREQ(def->os.type, "xen") || diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 88cd4c8..45bc398 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -174,8 +174,10 @@ virCapsPtr openvzCapsInit(void) 0, NULL) == NULL) goto no_memory; - return caps; + caps->defaultInitPath = "/sbin/init"; + + return caps; no_memory: virCapabilitiesFree(caps); return NULL; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 60f2dc2..6af4f75 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -857,13 +857,6 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; - if (vmdef->os.init == NULL) { - if (!(vmdef->os.init = strdup("/sbin/init"))) { - virReportOOMError(); - goto cleanup; - } - } - vm = virDomainFindByName(&driver->domains, vmdef->name); if (vm) { openvzError(VIR_ERR_OPERATION_FAILED, @@ -943,13 +936,6 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; - if (vmdef->os.init == NULL) { - if (!(vmdef->os.init = strdup("/sbin/init"))) { - virReportOOMError(); - goto cleanup; - } - } - vm = virDomainFindByName(&driver->domains, vmdef->name); if (vm) { openvzError(VIR_ERR_OPERATION_FAILED, -- 1.7.4.4

3 4

[libvirt] [PATCH] phyp: avoid a crash
by Eric Blake 14 May '11

14 May '11

This has been present since the introduction of phypAttachDevice in commit 444fd07a. * src/phyp/phyp_driver.c (phypAttachDevice): Don't dereference NULL. --- Found by clang, but the NULL dereference is very blatant. However, I'm worried that this patch, while solving the NULL deref, is dead wrong. In researching this patch, I also found a memory leak: phypDomainCreateAndStart mallocs a virDomainDefPtr and passes it phypBuildLpar, but phypBuildLpar neither stashes that memory into a hash table nor frees it. If it were to stash it into a table, then subsequent operations on the same domain should reuse that existing def, rather than malloc'ing one from scratch. Conversely, if the driver can reconstruct a def in entirety by reading lpar state, then def should be freed, and there should be a function to recreate a def at will. Mallocing a temporary def in functions like phypAttachDevice is probably the wrong thing to do, when really we want to get at the domain definition corresponding to the domain we are modifying. src/phyp/phyp_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 71a3d29..3b4235c 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1716,14 +1716,19 @@ phypAttachDevice(virDomainPtr domain, const char *xml) char *vios_name = NULL; char *profile = NULL; virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *domain_name = NULL; + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + goto cleanup; + } + domain_name = escape_specialcharacters(domain->name); if (domain_name == NULL) { goto cleanup; } def->os.type = strdup("aix"); -- 1.7.4.4

2 2

[libvirt] Potential problems in virFDStreamClose
by Matthias Bolte 14 May '11

14 May '11

virFDStreamClose calls virFDStreamFree that frees fdst, but virFDStreamClose access fdst->lock after that. virFDStreamClose doesn't destroy fdst->lock. static int virFDStreamClose(virStreamPtr st) { struct virFDStreamData *fdst = st->privateData; int ret; VIR_DEBUG("st=%p", st); if (!fdst) return 0; virMutexLock(&fdst->lock); ret = virFDStreamFree(fdst); st->privateData = NULL; virMutexUnlock(&fdst->lock); return ret; } Matthias

2 3

[libvirt] [PATCH] time_t is not a long on FreeBSD, need to add casts
by Matthias Bolte 14 May '11

14 May '11

--- src/conf/domain_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..875f90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9115,7 +9115,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec)); if (def->name == NULL) { virReportOOMError(); @@ -9126,7 +9126,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, if (!newSnapshot) { if (virXPathLong("string(./creationTime)", ctxt, - &def->creationTime) < 0) { + (long *)&def->creationTime) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; @@ -9192,7 +9192,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAddLit(&buf, " </parent>\n"); } virBufferAsprintf(&buf, " <creationTime>%ld</creationTime>\n", - def->creationTime); + (long)def->creationTime); virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); virBufferAddLit(&buf, " </domain>\n"); -- 1.7.0.4

3 4

[libvirt] [PATCH] command: Fix compilation on FreeBSD
by Matthias Bolte 14 May '11

14 May '11

kill, SIGTERM and SIGKILL require additional headers. --- src/util/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index b488d55..b2a873b 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -22,9 +22,11 @@ #include <config.h> #include <poll.h> +#include <signal.h> #include <stdarg.h> #include <stdlib.h> #include <sys/stat.h> +#include <sys/types.h> #include <sys/wait.h> #include "command.h" -- 1.7.0.4

4 5

[libvirt] [PATCH] virsh: Fix uninitialized variable warning
by Matthias Bolte 14 May '11

14 May '11

Reported on FreeBSD only. --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index fbeb7c8..3baa015 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11131,7 +11131,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) if (def->opts) { const vshCmdOptDef *opt; for (opt = def->opts; opt->name; opt++) { - const char *fmt; + const char *fmt = "%s"; switch (opt->type) { case VSH_OT_BOOL: fmt = "[--%s]"; -- 1.7.0.4

2 2

[libvirt] [PATCH] apparmor: Fix compilation by removing remains from virCommand conversion
by Matthias Bolte 14 May '11

14 May '11

Commit aaf20355b87d3bfda7579a7f6a4a978e848635c3 was incomplete here and missed to remove some parts. --- Pushing this one under the build breaker rule. src/security/security_apparmor.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 221e331..f47ded7 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -163,7 +163,7 @@ load_profile(virSecurityManagerPtr mgr, const char *fn, bool append) { - int rc = -1, status, ret; + int rc = -1; bool create = true; char *xml = NULL; virCommandPtr cmd; @@ -194,9 +194,6 @@ load_profile(virSecurityManagerPtr mgr, clean: VIR_FREE(xml); - VIR_FORCE_CLOSE(pipefd[0]); - VIR_FORCE_CLOSE(pipefd[1]); - return rc; } -- 1.7.0.4

1 0

[libvirt] [PATCH] configure: Fix mpath check on non-Linux systems
by Matthias Bolte 14 May '11

14 May '11

--- configure.ac | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index a2ce97e..4f5c2d7 100644 --- a/configure.ac +++ b/configure.ac @@ -1667,11 +1667,15 @@ if test "$with_storage_scsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) -if test "$with_storage_mpath" = "check" && test "$with_linux" = "yes"; then - with_storage_mpath=yes - - AC_DEFINE_UNQUOTED([WITH_STORAGE_MPATH], 1, - [whether mpath backend for storage driver is enabled]) +if test "$with_storage_mpath" = "check"; then + if test "$with_linux" = "yes"; then + with_storage_mpath=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_MPATH], 1, + [whether mpath backend for storage driver is enabled]) + else + with_storage_mpath=no + fi fi AM_CONDITIONAL([WITH_STORAGE_MPATH], [test "$with_storage_mpath" = "yes"]) -- 1.7.0.4

2 2