- Devel - libvirt List Archives

[libvirt] [PATCH] build: fix broken build
by Eric Blake 20 Jul '11

20 Jul '11

* src/libxl/libxl_driver.c (libxlDomainUndefineFlags): Use correct enum value. --- Pushing under the build-breaker rule. src/libxl/libxl_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 2e7197c..d52a8b6 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2766,7 +2766,7 @@ libxlDomainUndefineFlags(virDomainPtr dom, if (virFileExists(name)) { if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) { if (unlink(name) < 0) { - libxlError(VIR_ERR_INTERNAL_ERR, + libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to remove domain managed save image")); goto cleanup; } -- 1.7.4.4

1 1

[libvirt] [PATCH v4 1/6] undefine: Define the new API
by Osier Yang 20 Jul '11

20 Jul '11

This introduces a new API virDomainUndefineFlags to control the domain undefine process, as the existing API virDomainUndefine doesn't support flags. Currently only flag VIR_DOMAIN_UNDEFINE_MANAGED_SAVE is supported. If the domain has a managed save image, including VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in @flags will also remove that file, and omitting the flag will cause undefine process to fail. This patch also changes the behavior of virDomainUndefine, if the domain has a managed save image, the undefine will be refused. --- include/libvirt/libvirt.h.in | 10 +++++++ src/driver.h | 4 +++ src/libvirt.c | 60 +++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 5 +++ 4 files changed, 78 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 607b5bc..5f9f08a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1200,6 +1200,16 @@ int virDomainMemoryPeek (virDomainPtr dom, virDomainPtr virDomainDefineXML (virConnectPtr conn, const char *xml); int virDomainUndefine (virDomainPtr domain); + +typedef enum { + VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1, + + /* Future undefine control flags should come here. */ +} virDomainUndefineFlagsValues; + + +int virDomainUndefineFlags (virDomainPtr domain, + unsigned int flags); int virConnectNumOfDefinedDomains (virConnectPtr conn); int virConnectListDefinedDomains (virConnectPtr conn, char **const names, diff --git a/src/driver.h b/src/driver.h index 9d0d3de..4c4955f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -219,6 +219,9 @@ typedef virDomainPtr typedef int (*virDrvDomainUndefine) (virDomainPtr dom); typedef int + (*virDrvDomainUndefineFlags) (virDomainPtr dom, + unsigned int flags); +typedef int (*virDrvDomainSetVcpus) (virDomainPtr domain, unsigned int nvcpus); typedef int @@ -733,6 +736,7 @@ struct _virDriver { virDrvDomainCreateWithFlags domainCreateWithFlags; virDrvDomainDefineXML domainDefineXML; virDrvDomainUndefine domainUndefine; + virDrvDomainUndefineFlags domainUndefineFlags; virDrvDomainAttachDevice domainAttachDevice; virDrvDomainAttachDeviceFlags domainAttachDeviceFlags; virDrvDomainDetachDevice domainDetachDevice; diff --git a/src/libvirt.c b/src/libvirt.c index 39e2041..2f5241a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6374,7 +6374,13 @@ error: * virDomainUndefine: * @domain: pointer to a defined domain * - * Undefine a domain but does not stop it if it is running + * Undefine a domain. If the domain is running, it's converted to + * transient domain, without stopping it. If the domain is inactive, + * the domain configuration is removed. + * + * If the domain has a managed save image (see + * virDomainHasManagedSaveImage()), then the undefine will fail. See + * virDomainUndefineFlags() for more control. * * Returns 0 in case of success, -1 in case of error */ @@ -6413,6 +6419,58 @@ error: } /** + * virDomainUndefineFlags: + * @domain: pointer to a defined domain + * @flags: bitwise-or of supported virDomainUndefineFlagsValues + * + * Undefine a domain. If the domain is running, it's converted to + * transient domain, without stopping it. If the domain is inactive, + * the domain configuration is removed. + * + * If the domain has a managed save image (see virDomainHasManagedSaveImage()), + * then including VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in @flags will also remove + * that file, and omitting the flag will cause the undefine process to fail. + * + * Returns 0 in case of success, -1 in case of error + */ +int +virDomainUndefineFlags(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + conn = domain->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainUndefineFlags) { + int ret; + ret = conn->driver->domainUndefineFlags (domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virConnectNumOfDefinedDomains: * @conn: pointer to the hypervisor connection * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5f2541a..5cc480e 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -466,4 +466,9 @@ LIBVIRT_0.9.3 { virNodeGetMemoryStats; } LIBVIRT_0.9.2; +LIBVIRT_0.9.4 { + global: + virDomainUndefineFlags; +} LIBVIRT_0.9.3; + # .... define new API here using predicted next version number .... -- 1.7.6

2 12

[libvirt] RHEL guest shuts down after running anaconda instead of rebooting
by Eric 20 Jul '11

20 Jul '11

when i create a RHEL 6 guest using virt-install it will go through the process of installing the OS and want to reboot once finished. when it reboots the VM powers off and i have to start it manually. is there a way to prevent this? i can reboot guests once they're up and running and they'll come back up without issue, but they always shut down when i'm installing them. host OS is RHEL 6 x86_64.

1 0

[libvirt] CentOS/RHEL 6 RPMs repository
by Mohammed Naser 19 Jul '11

19 Jul '11

Hi list! First post here, thanks for a wonderful application, I have been around in #virt on IRC and just signed up today. My little contribution: We use CentOS 6 in production and we were not able to get access to the latest 0.9.3 RPMs and we needed some of the new features, so we compiled a bunch of RPMs to help those save time (and us). Please note that we're not responsible if it blows up your computer, etc. etc. wget http://repo.vexxhost.com/virt/virt.repo -O /etc/yum.repos.d/virt.repo yum install libvirt That should get you started on libvirt 0.9.3, there are a couple of unavailable dependencies that are also on the repo, so you should be good to go! If anyone has any issues, please let me know. Other than that, enjoy the repos! Also, if you don't trust us and want to compile the SRPMS yourself, they are also available -- if you are running i386 and compile a set, drop it by so we can help people as it is x86_64 RPMs only right now. Regards, -- Mohammed Naser — vexxhost -----------------------------------------------------

1 0

[libvirt] [PATCH] maint: fix spelling of Red Hat
by Eric Blake 19 Jul '11

19 Jul '11

* cfg.mk (sc_copyright_format): Add to rule. * src/util/files.h: Fix offenders. * src/util/files.c: Likewise. --- Pushing under the trivial rule. Noticed it because my upcoming edits to files.c triggered my editor's copyright update rule. The 'syntax-check' addition ensures that no one else makes the mistake. cfg.mk | 3 +++ src/util/files.c | 4 ++-- src/util/files.h | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index f98c94b..d243862 100644 --- a/cfg.mk +++ b/cfg.mk @@ -584,6 +584,9 @@ sc_copyright_format: @prohibit='Copyright [^(].*Red 'Hat \ halt='consistently use (C) in Red Hat copyright' \ $(_sc_search_regexp) + @prohibit='\<Red''Hat\>' \ + halt='spell Red Hat as two words' \ + $(_sc_search_regexp) # Some functions/macros produce messages intended solely for developers # and maintainers. Do not mark them for translation. diff --git a/src/util/files.c b/src/util/files.c index de4ae87..bef56b6 100644 --- a/src/util/files.c +++ b/src/util/files.c @@ -1,9 +1,9 @@ /* - * memory.c: safer file handling + * files.c: safer file handling * + * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright (C) 2010 IBM Corporation * Copyright (C) 2010 Stefan Berger - * Copyright (C) 2010 RedHat, Inc. * Copyright (C) 2010 Eric Blake * * This library is free software; you can redistribute it and/or diff --git a/src/util/files.h b/src/util/files.h index 5137fb3..8b681eb 100644 --- a/src/util/files.h +++ b/src/util/files.h @@ -1,7 +1,7 @@ /* * files.h: safer file handling * - * Copyright (C) 2010-2011 RedHat, Inc. + * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright (C) 2010 IBM Corporation * Copyright (C) 2010 Stefan Berger * Copyright (C) 2010 Eric Blake -- 1.7.4.4

1 0

[libvirt] [PATCH 0/8] Avoid filesystem cache pollution during virsh save
by Eric Blake 19 Jul '11

19 Jul '11

Saving a domain's state creates a large file, which risks polluting the filesystem cache and slowing down a system. If a system has a lot of domians simultaneously being saved (such as the libvirt-guests init script doing managed saves), then this can cause noticeable slowdown due to filesystem thrashing. This patch series has been successfully tested to do 'virsh save dom file --direct', with lsof(1) used to verify that O_DIRECT was in use, and the resulting file was successfully used with 'virsh restore file'. Still to come - wire up O_DIRECT on the 'virsh restore' path (yes, that means adding virDomainRestoreFlags - if only we had had the foresight to use flags everywhere). Wire up qemu.conf to allow the automatic use of --direct on automatic core dumps. Wire up libvirt-guests init script to allow the use of --direct. But I had enough in place to get the review started now. Also, this series demonstrates some of the points I was making about adding a new Flags API in this thread: https://www.redhat.com/archives/libvir-list/2011-July/msg00762.html This series requires and was tested on top of these (un-acked) patches: https://www.redhat.com/archives/libvir-list/2011-July/msg00670.html https://www.redhat.com/archives/libvir-list/2011-July/msg00675.html Eric Blake (8): save: document new public API save: wire up remote protocol save: wire up trivial saveFlags implementations save: add --direct flag to virsh save operations save: let iohelper handle inherited fd save: let iohelper work on O_DIRECT fds save: add virDirectFd wrapper type save: support O_DIRECT during qemu saves cfg.mk | 1 + configure.ac | 6 +- include/libvirt/libvirt.h.in | 14 +++ po/POTFILES.in | 1 + src/Makefile.am | 1 + src/driver.h | 6 ++ src/fdstream.c | 32 +++----- src/libvirt.c | 94 +++++++++++++++++++++- src/libvirt_private.syms | 6 ++ src/libvirt_public.syms | 5 + src/libxl/libxl_driver.c | 17 ++++- src/qemu/qemu_driver.c | 76 +++++++++++++----- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 11 +++- src/remote_protocol-structs | 6 ++ src/test/test_driver.c | 20 ++++- src/util/iohelper.c | 185 +++++++++++++++++++++++++++++++---------- src/util/virdirect.c | 149 +++++++++++++++++++++++++++++++++ src/util/virdirect.h | 37 +++++++++ src/vbox/vbox_tmpl.c | 19 ++++- src/xen/xen_driver.c | 17 ++++- tools/virsh.c | 12 +++- tools/virsh.pod | 17 +++- 23 files changed, 632 insertions(+), 101 deletions(-) create mode 100644 src/util/virdirect.c create mode 100644 src/util/virdirect.h -- 1.7.4.4

2 44

Re: [libvirt] mingw: test-poll pipe part fails
by Eric Blake 19 Jul '11

19 Jul '11

[adding libvirt] On 06/04/2011 12:24 AM, Paolo Bonzini wrote: > On Sat, Jun 4, 2011 at 00:37, Matthias Bolte > <matthias.bolte(a)googlemail.com> wrote: >> After testing a while and reading MSDN docs the problem seems to be >> that MsgWaitForMultipleObjects doesn't work on pipes. It doesn't >> actually wait but just returns immediately. Digging MSDN and googling >> about this suggest that there is no simple solution to this. > > Yes, Windows pipes are that broken. :( > > Using socketpair is a possibly good idea, but I would do it on > libvirtd only. I don't know exactly how libvirtd uses this pipe, but > perhaps it can be changed to an eventfd-like abstraction that can be > used with both Windows and Unix. This is similar to Eric's > suggestion, but without the pipe at all. It would also be a > libvirtd-specific suggestion. I'm wondering if the problem here is that libvirt is trying to use the pipe-to-self mechanism as a fundamental event loop idiom. That is, the reason libvirt is calling poll is in order to minimize CPU until something interesting happens, where interesting includes needing to wake up a helper thread to do an action inside locks in response to the receipt of a signal. Maybe you are on to something, and replacing all uses of pipe() with virPipeToSelf() (which uses pipe() for efficiency on Linux, but socketpair() on mingw), would allow libvirt to continue to use the pipe-to-self idiom while also using fds that can actually be poll'd on mingw. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

4 6

[libvirt] enumerating disk devices
by Serge E. Hallyn 19 Jul '11

19 Jul '11

Hi, Some people appear to have autostart VMs residing on slow storage. If libvirtd starts too early, it'll simply fail trying to start those VMs. It'd be nice to know when all the storage on which autostart containers depend becomes available, so as to safely start libvirt. The python script appended below is one approach. It could be called as something like for f in /etc/libvirt/qemu/autostart/* /etc/libvirt/lxc/autostart/*; do for d in `libvirt_list_storage.py $f`; do if [ ! -r $d ]; then echo "storage not yet ready" exit 1 fi done done echo "starting libvirtd" libvirtd (Realistically I would have this running in an upstart job which is 'start on mounted' and, if all storage available, emits a 'libvirt-storage-up' event). Are there better ways to go about this? thanks, -serge #!/usr/bin/python import sys from xml.dom.minidom import parse dom = parse(sys.argv[1]) def getText(nodelist): rc = [] for node in nodelist: if node.nodeType == node.TEXT_NODE: rc.append(node.data) return ''.join(rc) def print_domname(dom): #return print "Domain %s\n" % getText(dom.childNodes) def liststorages(dom): print_domname(dom.getElementsByTagName("name")[0]) disks = dom.getElementsByTagName("disk") list_disks(disks) filesystems = dom.getElementsByTagName('filesystem') list_filesystems(filesystems) def list_filesystems(fslist): for fs in fslist: if not fs.hasAttribute('type'): continue if fs.getAttribute('type') != 'mount': continue dirs = fs.getElementsByTagName("source") if len(dirs) == 0: continue if not dirs[0].getAttribute('dir'): continue print "dir: %s\n" % dirs[0].getAttribute('dir') def list_disks(disks): for disk in disks: if not disk.hasAttribute('type'): continue if disk.getAttribute('type') == 'file': list_disk_type_file(disk) elif disk.getAttribute('type') == 'block': list_disk_type_block(disk) else: # not supported continue def list_disk_type_file(disk): disksrcs = disk.getElementsByTagName("source") if len(disksrcs) == 0: return if not disksrcs[0].hasAttribute('file'): return print "file: %s\n" % disksrcs[0].getAttribute('file') def list_disk_type_block(disk): devsrscs = disk.getElementsByTagName("source") if len(devsrscs) == 0: return if not devsrscs[0].hasAttribute('dev'): return print "dev: %s\n" % devsrscs[0].getAttribute('dev') liststorages(dom)

1 0

[libvirt] [PATCH 0/4] Some RPC event handler ref counting fixes
by Daniel P. Berrange 19 Jul '11

19 Jul '11

Wen previously posted a report of a crash with migration. I could not reproduce it at the time, but since then someone else did and pointed out that it only occurs when TLS is in use https://www.redhat.com/archives/libvir-list/2011-July/msg00547.html This series of patches make the event handler reference counting more robust to avoid this problem

2 9

[libvirt] [PATCH 1/2] Add some basic sanity checking of certificates before use
by Daniel P. Berrange 19 Jul '11

19 Jul '11

If the libvirt daemon or libvirt client is configured with bogus certificates, it is very unhelpful to only find out about this when a TLS connection is actually attempted. Not least because the error messages you get back for failures are incredibly obscure. This adds some basic sanity checking of certificates at the time the virNetTLSContext object is created. This is at libvirt startup, or when creating a virNetClient instance. This checks that the certificate expiry/start dates are valid and that the certificate is actually signed by the CA that is loaded. * src/rpc/virnettlscontext.c: Add certificate sanity checks --- src/rpc/virnettlscontext.c | 149 ++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 145 insertions(+), 4 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 4fa6fbb..b0dcfe5 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -67,6 +67,7 @@ struct _virNetTLSSession { bool handshakeComplete; + bool isServer; char *hostname; gnutls_session_t session; virNetTLSSessionWriteFunc writeFunc; @@ -95,6 +96,134 @@ static void virNetTLSLog(int level, const char *str) { VIR_DEBUG("%d %s", level, str); } + +static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer, + const char *certFile) +{ + gnutls_datum_t data; + gnutls_x509_crt_t cert = NULL; + char *buf = NULL; + int ret = -1; + time_t now; + + if ((now = time(NULL)) == ((time_t)-1)) { + virReportSystemError(errno, "%s", + _("cannot get current time")); + goto cleanup; + } + + if (gnutls_x509_crt_init(&cert) < 0) { + virNetError(VIR_ERR_SYSTEM_ERROR, "%s", + _("Unable to initialize certificate")); + goto cleanup; + } + + if (virFileReadAll(certFile, (1<<16), &buf) < 0) + goto cleanup; + + data.data = (unsigned char *)buf; + data.size = strlen(buf); + + if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) { + virNetError(VIR_ERR_SYSTEM_ERROR, isServer ? + _("Unable to import server certificate %s") : + _("Unable to import client certificate %s"), + certFile); + goto cleanup; + } + + if (gnutls_x509_crt_get_expiration_time(cert) < now) { + virNetError(VIR_ERR_SYSTEM_ERROR, isServer ? + _("The server certificate %s has expired") : + _("The client certificate %s has expired"), + certFile); + goto cleanup; + } + + if (gnutls_x509_crt_get_activation_time(cert) > now) { + virNetError(VIR_ERR_SYSTEM_ERROR, isServer ? + _("The server certificate %s is not yet active") : + _("The client certificate %s is not yet active"), + certFile); + goto cleanup; + } + + ret = 0; + +cleanup: + if (ret != 0) { + gnutls_x509_crt_deinit(cert); + cert = NULL; + } + VIR_FREE(buf); + return cert; +} + + +static int virNetTLSContextSanityCheckCredentials(bool isServer, + const char *cacertFile, + const char *certFile) +{ + gnutls_x509_crt_t cert = NULL; + gnutls_x509_crt_t cacert = NULL; + int ret = -1; + unsigned int status; + + if (access(certFile, R_OK) == 0) { + if (!(cert = virNetTLSContextSanityCheckCert(isServer, certFile))) + goto cleanup; + } + if (access(cacertFile, R_OK) == 0) { + if (!(cacert = virNetTLSContextSanityCheckCert(isServer, cacertFile))) + goto cleanup; + } + + if (cert && cacert) { + if (gnutls_x509_crt_list_verify(&cert, 1, + &cacert, 1, + NULL, 0, + 0, &status) < 0) { + virNetError(VIR_ERR_SYSTEM_ERROR, "%s", isServer ? + _("Unable to verify server certificate against CA certificate") : + _("Unable to verify client certificate against CA certificate")); + goto cleanup; + } + + if (status != 0) { + const char *reason = _("Invalid certificate"); + + if (status & GNUTLS_CERT_INVALID) + reason = _("The certificate is not trusted."); + + if (status & GNUTLS_CERT_SIGNER_NOT_FOUND) + reason = _("The certificate hasn't got a known issuer."); + + if (status & GNUTLS_CERT_REVOKED) + reason = _("The certificate has been revoked."); + +#ifndef GNUTLS_1_0_COMPAT + if (status & GNUTLS_CERT_INSECURE_ALGORITHM) + reason = _("The certificate uses an insecure algorithm"); +#endif + + virNetError(VIR_ERR_SYSTEM_ERROR, + _("Our own certificate %s failed validation against %s: %s"), + certFile, cacertFile, reason); + goto cleanup; + } + } + + ret = 0; + +cleanup: + if (cert) + gnutls_x509_crt_deinit(cert); + if (cacert) + gnutls_x509_crt_deinit(cacert); + return ret; +} + + static int virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt, bool isServer, const char *cacert, @@ -217,6 +346,10 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, goto error; } + if (requireValidCert && + virNetTLSContextSanityCheckCredentials(isServer, cacert, cert) < 0) + goto error; + if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, cert, key) < 0) goto error; @@ -574,15 +707,21 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, } if (gnutls_x509_crt_get_expiration_time(cert) < now) { - virNetError(VIR_ERR_SYSTEM_ERROR, "%s", - _("The client certificate has expired")); + /* Warning is reversed from what you expect, since with + * this code it is the Server checking the client and + * vica-verca */ + virNetError(VIR_ERR_SYSTEM_ERROR, "%s", sess->isServer ? + _("The client certificate has expired") : + _("The server certificate has expired")); gnutls_x509_crt_deinit(cert); goto authdeny; } if (gnutls_x509_crt_get_activation_time(cert) > now) { - virNetError(VIR_ERR_SYSTEM_ERROR, "%s", - _("The client certificate is not yet active")); + /* client/server order reversed. see above */ + virNetError(VIR_ERR_SYSTEM_ERROR, "%s", sess->isServer ? + _("The client certificate is not yet active") : + _("The server certificate is not yet active")); gnutls_x509_crt_deinit(cert); goto authdeny; } @@ -756,6 +895,8 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, gnutls_transport_set_pull_function(sess->session, virNetTLSSessionPull); + sess->isServer = ctxt->isServer; + return sess; error: -- 1.7.4.4

2 4

Re: [libvirt] [virt-tools-list] [PATCH virt-manager v4] Add inspection to virt-manager
by Richard W.M. Jones 19 Jul '11

19 Jul '11

On Tue, Jul 19, 2011 at 10:46:49AM -0400, Cole Robinson wrote: > Maybe we can cache the png data per detected OS value rather than per > VM? Not sure if that collides with licensing issues, but would likely > mean storing less data on disk. You can't do that without getting into trademark issues. The icon that is displayed must have come from precisely the same guest. The icons are not large anyhow. One thing I meant to ask about Dan's proposal: > > $HOME/.local/libvirt/$CONN_URI/$DOMAIN_UUID/screenshot.png > > $HOME/.local/libvirt/$CONN_URI/$DOMAIN_UUID/icon.png > > $HOME/.local/libvirt/$CONN_URI/$DOMAIN_UUID/osinfo.json Do we need the connection URI? Isn't the dom UUID unique enough? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

3 2

[libvirt] [PATCH] Fix reporting of errors for p2p migration
by Daniel P. Berrange 19 Jul '11

19 Jul '11

Starting/ending jobs when closing the connection may reset any error which was reported earlier in p2p migration. We must save the original error before doing so. This means we can also just call virConnectClose as normal, instead of virUnrefConnect * src/qemu/qemu_migration.c: Preserve errors in p2p migration --- src/qemu/qemu_migration.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index dfa80e3..1803b9f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2141,6 +2141,8 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, int ret = -1; virConnectPtr dconn = NULL; bool p2p; + virErrorPtr orig_err = NULL; + VIR_DEBUG("driver=%p, sconn=%p, vm=%p, xmlin=%s, dconnuri=%s, " "uri=%s, flags=%lx, dname=%s, resource=%lu", driver, sconn, vm, NULLSTR(xmlin), NULLSTR(dconnuri), @@ -2191,10 +2193,14 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, dconnuri, flags, dname, resource); cleanup: - /* don't call virConnectClose(), because that resets any pending errors */ + orig_err = virSaveLastError(); qemuDomainObjEnterRemoteWithDriver(driver, vm); - virUnrefConnect(dconn); + virConnectClose(dconn); qemuDomainObjExitRemoteWithDriver(driver, vm); + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } return ret; } -- 1.7.4.4

2 1

Re: [libvirt] [virt-tools-list] [PATCH virt-manager v4] Add inspection to virt-manager
by Richard W.M. Jones 19 Jul '11

19 Jul '11

On Tue, Jul 19, 2011 at 03:40:53PM +0100, Daniel P. Berrange wrote: > On Tue, Jul 19, 2011 at 10:17:02AM -0400, Cole Robinson wrote: > > On 07/19/2011 03:49 AM, Richard W.M. Jones wrote: > > > On Mon, Jul 18, 2011 at 05:41:00PM -0400, Cole Robinson wrote: > > >> On 07/18/2011 02:53 PM, Richard W.M. Jones wrote: > > >>> This updates 1/4 with the fixes you suggested. Also, all check-pylint > > >>> warnings and errors have been fixed. > > >>> > > >>> Rich. > > >>> > > >> > > >> Thanks! Pushed the series. > > > > > > In reply to your comment on IRC: > > > > > > crobinso> rjones: so in the default usage of virt-manager in fedora, > > > the guest inspection probably won't work since we > > > save disk images in /var/lib/libvirt/images/, which > > > regular user doesn't have access to. > > > crobinso> rjones: that's not entirely clear from feature page. > > > crobinso> rjones: I'm thinking of adding a disk path access check in > > > the inspection thread, to avoid flooding the logs > > > with errors if we can't even read the disk > > > image. that should be safe to do? > > > > > > I always run virt-manager as root (or from sudo) so this hasn't been > > > an issue. What user does virt-manager run as normally? > > > > > > > I think most common usage is just running virt-manager as the logged in > > user, using policykit to authenticate the libvirt connection so the rest > > of the app doesn't have root privs. > > > > > AFAICT if there's no access to the disks, then the call to either > > > g.add_drive_opts or g.launch will throw an exception. But the > > > inspection._vmseen hash will mean this will only happen once per > > > domain per run of virt-manager. > > > > > > On an unrelated note: I think we need to cache inspection between runs > > > of virt-manager. Does virt-manager currently store permanent state (I > > > assume it must do - ie. list of connections), and where? > > > > > > > We store config like that in gconf, though I don't think sticking > > largish data like list of applications or a png in there is a good idea. > > hostname + os info could be cached, though ideally the latter would be > > stored in libvirt XML at some point. > > Agreed, it would be desirable to cache the PNGs in $HOME/.virt-manager > though, perhaps as > > $HOME/.virt-manager/icons/$CONN_URI/$DOMAIN_UUID > > To avoid growing without bound, probably want to have virt-manager > purge files in that directory on startup, if the PNG is older than > 3 months and the associated connection or domain no longer exists. > ie don't immediately purge them, since a user might later re-add a > connection or re-create a VM. > > Another thought though is that we've also got some work going on a > little plugin for gnome-shell to capture & display screenshots of > VMs. It might be nice for virt-manager to take advantage of this > too, while also the shell plugin might like the icon image. So > perhaps we should declare a standard location for storing assets > related to a VM, which are expensive to extract/fetch. > > $HOME/.local/libvirt/$CONN_URI/$DOMAIN_UUID/screenshot.png > $HOME/.local/libvirt/$CONN_URI/$DOMAIN_UUID/icon.png > $HOME/.local/libvirt/$CONN_URI/$DOMAIN_UUID/osinfo.json > > or something like that +1 CC-ing to libvir-list. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

1 0

[libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
by Adam Litke 19 Jul '11

19 Jul '11

In light of discussion on V1 of this API, here is V2 of the next generation BlockPull API. In this series we make the abort and info operations more generic so that they may apply to future block jobs (such as compression, live block copy, etc). We also add in a mechanism to set the maximum bandwidth of an operation. Changes since V1: - Make virDomainBlockPullAbort() and virDomainGetBlockPullInfo() into a generic BlockJob interface. - Added virDomainBlockJobSetSpeed() - Rename VIR_DOMAIN_EVENT_ID_BLOCK_PULL event to fit into block job API - Add bandwidth argument to virDomainBlockPull() Summary of changes since first generation patch series: - Qemu dropped incremental streaming so remove libvirt incremental BlockPull() API - Rename virDomainBlockPullAll() to virDomainBlockPull() - Changes required to qemu monitor handlers for changed command names -- To help speed the provisioning process for large domains, new QED disks are created with backing to a template image. These disks are configured with copy on read such that blocks that are read from the backing file are copied to the new disk. This reduces I/O over a potentially costly path to the backing image. In such a configuration, there is a desire to remove the dependency on the backing image as the domain runs. To accomplish this, qemu will provide an interface to perform sequential copy on read operations during normal VM operation. Once all data has been copied, the disk image's link to the backing file is removed. The virDomainBlockPull API family brings this functionality to libvirt. virDomainBlockPull() instructs the hypervisor to stream the entire device in the background. Progress of this operation can be checked with the function virDomainBlockJobInfo(). An ongoing stream can be cancelled with virDomainBlockJobAbort(). virDomainBlockJobSetSpeed() allows you to limit the bandwidth that the operation may consume. An event (VIR_DOMAIN_EVENT_ID_BLOCK_JOB) will be emitted when a disk has been fully populated or if a BlockPull() operation was terminated due to an error. This event is useful to avoid polling on virDomainBlockJobInfo() for completion and could also be used by the security driver to revoke access to the backing file when it is no longer needed. /* * BlockJob API */ /** * virDomainBlockJobType: * * VIR_DOMAIN_BLOCK_JOB_TYPE_PULL: Block Pull (virDomainBlockPull) */ typedef enum { VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = -1, VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 0, } virDomainBlockJobType; /* An iterator for monitoring block job operations */ typedef unsigned long long virDomainBlockJobCursor; typedef struct _virDomainBlockJobInfo virDomainBlockJobInfo; struct _virDomainBlockJobInfo { virDomainBlockJobType type; /* * The following fields provide an indication of block job progress. @cur * indicates the current position and will be between 0 and @end. @end is * the final cursor position for this operation and represents completion. * To approximate progress, divide @cur by @end. */ virDomainBlockJobCursor cur; virDomainBlockJobCursor end; }; typedef virDomainBlockJobInfo *virDomainBlockJobInfoPtr; /** * virDomainBlockJobAbort: * @dom: pointer to domain object * @path: fully-qualified filename of disk * @flags: currently unused, for future extension * * Cancel the active block job on the given disk. * * Returns -1 in case of failure, 0 when successful. */ int virDomainBlockJobAbort(virDomainPtr dom, const char *path, unsigned int flags); /** * virDomainGetBlockJobInfo: * @dom: pointer to domain object * @path: fully-qualified filename of disk * @info: pointer to a virDomainBlockJobInfo structure * @flags: currently unused, for future extension * * Request block job information for the given disk. If an operation is active * @info will be updated with the current progress. * * Returns -1 in case of failure, 0 when successful. */ int virDomainGetBlockJobInfo(virDomainPtr dom, const char *path, virDomainBlockJobInfoPtr info, unsigned int flags); /** * virDomainBlockJobSetSpeed: * @dom: pointer to domain object * @path: fully-qualified filename of disk * @bandwidth: specify bandwidth limit in Mbps * @flags: currently unused, for future extension * * Set the maximimum allowable bandwidth that a block job may consume. If * bandwidth is 0, the limit will revert to the hypervisor default. * * Returns -1 in case of failure, 0 when successful. */ int virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); /** * virConnectDomainEventBlockJobStatus: * * The final status of a block job */ typedef enum { VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0, VIR_DOMAIN_BLOCK_JOB_FAILED = 1, } virConnectDomainEventBlockJobStatus; /** * virConnectDomainEventBlockPullCallback: * @conn: connection object * @dom: domain on which the event occurred * @path: fully-qualified filename of the affected disk * @type: type of block job (virDomainBlockJobType) * @status: final status of the operation (virConnectDomainEventBlockPullStatus) * @opaque: callback context * * The callback signature to use when registering for an event of type * VIR_DOMAIN_EVENT_ID_BLOCK_PULL with virConnectDomainEventRegisterAny() */ typedef void (*virConnectDomainEventBlockPullCallback)(virConnectPtr conn, virDomainPtr dom, const char *path, int type, int status, void *opaque); /** * virDomainBlockPull: * @dom: pointer to domain object * @path: Fully-qualified filename of disk * @bandwidth: (optional) specify copy bandwidth limit in Mbps * @flags: currently unused, for future extension * * 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. This function pulls data for the entire device in the background. * Progress of the operation can be checked with virDomainGetBlockJobInfo() and * the operation can be aborted with virDomainBlockJobAbort(). When finished, * an asynchronous event is raised to indicate the final status. * * The maximum bandwidth (in Mbps) that will be used to do the copy can be * specified with the bandwidth parameter. If set to 0, libvirt will choose a * suitable default. Some hypervisors do not support this feature and will * return an error if bandwidth is not 0. * * Returns 0 if the operation has started, -1 on failure. */ int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); -- Adam Litke IBM Linux Technology Center

2 1

[libvirt] [PATCH] Quieten build & ensure API build scripts exit with non-zero status
by Daniel P. Berrange 19 Jul '11

19 Jul '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The current API build scripts will continue and exit with a zero status even if they find problems. This has been the cause of many build problems, or hidden build errors, in the past. Change the scripts so they always exit with a non-zero status for any problems they do not understand. Also turn off all debug output by default so they respect $(AM_V_GEN) * docs/Makefile.am: Use $(AM_V_GEN) for API/HTML scripts * docs/apibuild.py, python/generator.py: Exit with non-zero status if problems are found. Also be silent, not outputting any debug messages. * src/Makefile.am: Use $(AM_V_GEN) for ESX generator * python/Makefile.am: Tweak rule --- docs/Makefile.am | 11 ++---- docs/apibuild.py | 95 ++++++++++++++++++++++++++++++-------------------- python/Makefile.am | 6 ++-- python/generator.py | 16 +++++--- src/Makefile.am | 2 +- 5 files changed, 75 insertions(+), 55 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 7ff94a0..4ba6b9d 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -153,21 +153,18 @@ internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in html/index.html: libvirt-api.xml newapi.xsl page.xsl sitemap.html.in - -@if [ -x $(XSLTPROC) ] ; then \ - echo "Rebuilding the HTML pages from the XML API" ; \ + $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \ $(XSLTPROC) --nonet -o $(srcdir)/ \ - $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi - -@if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ + $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi ; \ + if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \ if $(XMLCATALOG) '$(XML_CATALOG_FILE)' "-//W3C//DTD XHTML 1.0 Strict//EN" \ > /dev/null ; then \ - echo "Validating the resulting XHTML pages" ; \ SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \ $(XMLLINT) --catalogs --nonet --valid --noout $(srcdir)/html/*.html ; \ else echo "missing XHTML1 DTD" ; fi ; fi $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl) - -@echo Rebuilding devhelp files - -@if [ -x $(XSLTPROC) ] ; then \ + $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \ $(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \ $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml ; fi diff --git a/docs/apibuild.py b/docs/apibuild.py index f31a853..cb273dc 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -11,6 +11,8 @@ import os, sys import string import glob +quiet=1 +warnings=0 debug=0 debugsym=None @@ -95,7 +97,7 @@ class identifier: self.conditionals = None else: self.conditionals = conditionals[:] - if self.name == debugsym: + if self.name == debugsym and not quiet: print "=> define %s : %s" % (debugsym, (module, type, info, extra, conditionals)) @@ -155,7 +157,7 @@ class identifier: def update(self, header, module, type = None, info = None, extra=None, conditionals=None): - if self.name == debugsym: + if self.name == debugsym and not quiet: print "=> update %s : %s" % (debugsym, (module, type, info, extra, conditionals)) if header != None and self.header == None: @@ -203,7 +205,7 @@ class index: if d != None and name != None and type != None: self.references[name] = d - if name == debugsym: + if name == debugsym and not quiet: print "New ref: %s" % (d) return d @@ -244,9 +246,9 @@ class index: elif type == "macro": self.macros[name] = d else: - print "Unable to register type ", type + self.warning("Unable to register type ", type) - if name == debugsym: + if name == debugsym and not quiet: print "New symbol: %s" % (d) return d @@ -260,8 +262,8 @@ class index: if self.macros.has_key(id): del self.macros[id] if self.functions.has_key(id): - print "function %s from %s redeclared in %s" % ( - id, self.functions[id].header, idx.functions[id].header) + self.warning("function %s from %s redeclared in %s" % ( + id, self.functions[id].header, idx.functions[id].header)) else: self.functions[id] = idx.functions[id] self.identifiers[id] = idx.functions[id] @@ -273,15 +275,15 @@ class index: if self.macros.has_key(id): del self.macros[id] if self.variables.has_key(id): - print "variable %s from %s redeclared in %s" % ( - id, self.variables[id].header, idx.variables[id].header) + self.warning("variable %s from %s redeclared in %s" % ( + id, self.variables[id].header, idx.variables[id].header)) else: self.variables[id] = idx.variables[id] self.identifiers[id] = idx.variables[id] for id in idx.structs.keys(): if self.structs.has_key(id): - print "struct %s from %s redeclared in %s" % ( - id, self.structs[id].header, idx.structs[id].header) + self.warning("struct %s from %s redeclared in %s" % ( + id, self.structs[id].header, idx.structs[id].header)) else: self.structs[id] = idx.structs[id] self.identifiers[id] = idx.structs[id] @@ -294,8 +296,8 @@ class index: self.identifiers[id] = idx.unions[id] for id in idx.typedefs.keys(): if self.typedefs.has_key(id): - print "typedef %s from %s redeclared in %s" % ( - id, self.typedefs[id].header, idx.typedefs[id].header) + self.warning("typedef %s from %s redeclared in %s" % ( + id, self.typedefs[id].header, idx.typedefs[id].header)) else: self.typedefs[id] = idx.typedefs[id] self.identifiers[id] = idx.typedefs[id] @@ -311,15 +313,15 @@ class index: if self.enums.has_key(id): continue if self.macros.has_key(id): - print "macro %s from %s redeclared in %s" % ( - id, self.macros[id].header, idx.macros[id].header) + self.warning("macro %s from %s redeclared in %s" % ( + id, self.macros[id].header, idx.macros[id].header)) else: self.macros[id] = idx.macros[id] self.identifiers[id] = idx.macros[id] for id in idx.enums.keys(): if self.enums.has_key(id): - print "enum %s from %s redeclared in %s" % ( - id, self.enums[id].header, idx.enums[id].header) + self.warning("enum %s from %s redeclared in %s" % ( + id, self.enums[id].header, idx.enums[id].header)) else: self.enums[id] = idx.enums[id] self.identifiers[id] = idx.enums[id] @@ -330,10 +332,10 @@ class index: # check that function condition agrees with header if idx.functions[id].conditionals != \ self.functions[id].conditionals: - print "Header condition differs from Function for %s:" \ - % id - print " H: %s" % self.functions[id].conditionals - print " C: %s" % idx.functions[id].conditionals + self.warning("Header condition differs from Function for %s:" \ + % id) + self.warning(" H: %s" % self.functions[id].conditionals) + self.warning(" C: %s" % idx.functions[id].conditionals) up = idx.functions[id] self.functions[id].update(None, up.module, up.type, up.info, up.extra) # else: @@ -356,12 +358,13 @@ class index: def analyze(self): - self.analyze_dict("functions", self.functions) - self.analyze_dict("variables", self.variables) - self.analyze_dict("structs", self.structs) - self.analyze_dict("unions", self.unions) - self.analyze_dict("typedefs", self.typedefs) - self.analyze_dict("macros", self.macros) + if not quiet: + self.analyze_dict("functions", self.functions) + self.analyze_dict("variables", self.variables) + self.analyze_dict("structs", self.structs) + self.analyze_dict("unions", self.unions) + self.analyze_dict("typedefs", self.typedefs) + self.analyze_dict("macros", self.macros) class CLexer: """A lexer for the C language, tokenize the input by reading and @@ -621,6 +624,8 @@ class CParser: info, extra, self.conditionals) def warning(self, msg): + global warnings + warnings = warnings + 1 if self.no_error: return print msg @@ -1802,7 +1807,8 @@ class CParser: return token def parse(self): - self.warning("Parsing %s" % (self.filename)) + if not quiet: + print "Parsing %s" % (self.filename) token = self.token() while token != None: if token[0] == 'name': @@ -1869,7 +1875,8 @@ class docBuilder: pass def analyze(self): - print "Project %s : %d headers, %d modules" % (self.name, len(self.headers.keys()), len(self.modules.keys())) + if not quiet: + print "Project %s : %d headers, %d modules" % (self.name, len(self.headers.keys()), len(self.modules.keys())) self.idx.analyze() def scanHeaders(self): @@ -1995,7 +2002,7 @@ class docBuilder: else: output.write(" <field name='%s' type='%s' info='%s'/>\n" % (field[1] , field[0], desc)) except: - print "Failed to serialize struct %s" % (name) + self.warning("Failed to serialize struct %s" % (name)) output.write(" </struct>\n") else: output.write("/>\n"); @@ -2023,7 +2030,7 @@ class docBuilder: def serialize_function(self, output, name): id = self.idx.functions[name] - if name == debugsym: + if name == debugsym and not quiet: print "=>", id output.write(" <%s name='%s' file='%s' module='%s'>\n" % (id.type, @@ -2059,7 +2066,7 @@ class docBuilder: output.write(" <arg name='%s' type='%s' info='%s'/>\n" % (param[1], param[0], escape(param[2]))) self.indexString(name, param[2]) except: - print "Failed to save function %s info: " % name, `id.info` + self.warning("Failed to save function %s info: " % name, `id.info`) output.write(" </%s>\n" % (id.type)) def serialize_exports(self, output, file): @@ -2074,7 +2081,7 @@ class docBuilder: escape(dict.info[data]), string.lower(data))) except: - print "Header %s lacks a %s description" % (module, data) + self.warning("Header %s lacks a %s description" % (module, data)) if dict.info.has_key('Description'): desc = dict.info['Description'] if string.find(desc, "DEPRECATED") != -1: @@ -2287,7 +2294,8 @@ class docBuilder: def serialize(self): filename = "%s/%s-api.xml" % (self.path, self.name) - print "Saving XML description %s" % (filename) + if not quiet: + print "Saving XML description %s" % (filename) output = open(filename, "w") output.write('<?xml version="1.0" encoding="ISO-8859-1"?>\n') output.write("<api name='%s'>\n" % self.name) @@ -2323,7 +2331,8 @@ class docBuilder: output.close() filename = "%s/%s-refs.xml" % (self.path, self.name) - print "Saving XML Cross References %s" % (filename) + if not quiet: + print "Saving XML Cross References %s" % (filename) output = open(filename, "w") output.write('<?xml version="1.0" encoding="ISO-8859-1"?>\n') output.write("<apirefs name='%s'>\n" % self.name) @@ -2336,7 +2345,8 @@ def rebuild(): builder = None srcdir = os.environ["srcdir"] if glob.glob(srcdir + "/../src/libvirt.c") != [] : - print "Rebuilding API description for libvirt" + if not quiet: + print "Rebuilding API description for libvirt" dirs = [srcdir + "/../src", srcdir + "/../src/util", srcdir + "/../include/libvirt"] @@ -2344,12 +2354,13 @@ def rebuild(): dirs.append("../include/libvirt") builder = docBuilder("libvirt", srcdir, dirs, []) elif glob.glob("src/libvirt.c") != [] : - print "Rebuilding API description for libvirt" + if not quiet: + print "Rebuilding API description for libvirt" builder = docBuilder("libvirt", srcdir, ["src", "src/util", "include/libvirt"], []) else: - print "rebuild() failed, unable to guess the module" + self.warning("rebuild() failed, unable to guess the module") return None builder.scan() builder.analyze() @@ -2368,5 +2379,13 @@ if __name__ == "__main__": if len(sys.argv) > 1: debug = 1 parse(sys.argv[1]) + if warnings > 0: + sys.exit(2) + else: + sys.exit(0) else: rebuild() + if warnings > 0: + sys.exit(2) + else: + sys.exit(0) diff --git a/python/Makefile.am b/python/Makefile.am index 0edb3e4..a4c9a6b 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -53,11 +53,11 @@ GENERATED= libvirt-export.c \ libvirt.h \ libvirt.py -generated.stamp: $(srcdir)/$(GENERATE) $(API_DESC) - $(PYTHON) $(srcdir)/$(GENERATE) $(PYTHON) +$(GENERATE).stamp: $(srcdir)/$(GENERATE) $(API_DESC) + $(AM_V_GEN)$(PYTHON) $(srcdir)/$(GENERATE) $(PYTHON) && \ touch $@ -$(GENERATED): generated.stamp +$(GENERATED): $(GENERATE).stamp $(libvirtmod_la_OBJECTS): $(GENERATED) diff --git a/python/generator.py b/python/generator.py index c27ff73..d3a3616 100755 --- a/python/generator.py +++ b/python/generator.py @@ -11,6 +11,8 @@ import sys import string import re +quiet=1 + if __name__ == "__main__": # launched as a script srcPref = os.path.dirname(sys.argv[0]) @@ -617,7 +619,8 @@ def buildStubs(): sys.exit(1) n = len(functions.keys()) - print "Found %d functions in libvirt-api.xml" % (n) + if not quiet: + print "Found %d functions in libvirt-api.xml" % (n) py_types['pythonObject'] = ('O', "pythonObject", "pythonObject", "pythonObject") try: @@ -629,8 +632,8 @@ def buildStubs(): except IOError, msg: print file, ":", msg - - print "Found %d functions in libvirt-override-api.xml" % ( + if not quiet: + print "Found %d functions in libvirt-override-api.xml" % ( len(functions.keys()) - n) nb_wrap = 0 failed = 0 @@ -662,20 +665,21 @@ def buildStubs(): export.close() wrapper.close() - print "Generated %d wrapper functions" % nb_wrap + if not quiet: + print "Generated %d wrapper functions" % nb_wrap if unknown_types: print "Missing type converters: " for type in unknown_types.keys(): print "%s:%d " % (type, len(unknown_types[type])), - print - for f in functions_failed: print "ERROR: failed %s" % f if failed > 0: return -1 + if len(unknown_types) > 0: + return -1 return 0 ####################################################################### diff --git a/src/Makefile.am b/src/Makefile.am index 54b1ca0..875a43d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -796,7 +796,7 @@ endif BUILT_SOURCES += $(ESX_DRIVER_GENERATED) $(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input $(srcdir)/esx/esx_vi_generator.py - -srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py if WITH_ESX if WITH_DRIVER_MODULES -- 1.7.6

3 5

[libvirt] [PATCH] tests: Fix virshtest failure after dominfo changed
by Osier Yang 19 Jul '11

19 Jul '11

Caused by the new changed "dominfo" command. Pushed under trival rule. --- tests/virshtest.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tests/virshtest.c b/tests/virshtest.c index 0932efd..e22e582 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -22,6 +22,7 @@ Max memory: 261072 kB\n\ Used memory: 131072 kB\n\ Persistent: yes\n\ Autostart: disable\n\ +Managed save: unknown\n\ \n"; static const char *domuuid_fc4 = DOM_UUID "\n\n"; static const char *domid_fc4 = "2\n\n"; -- 1.7.6

1 0

[libvirt] [PATCH v2] util: avoid fds leak when virEventPollAddHandle fail
by ajia＠redhat.com 19 Jul '11

19 Jul '11

* src/util/event_poll.c: only fix file descriptors leak on when virEventPollAddHandle fail on virEventPollInit function. Detected in valgrind run: ==1254== ==1254== FILE DESCRIPTORS: 6 open at exit. ==1254== Open file descriptor 5: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== ==1254== Open file descriptor 4: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== * how to reproduce? % valgrind -v --track-fds=yes virsh list --- src/util/event_poll.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/util/event_poll.c b/src/util/event_poll.c index 285ba50..e2ae3a6 100644 --- a/src/util/event_poll.c +++ b/src/util/event_poll.c @@ -36,6 +36,7 @@ #include "event_poll.h" #include "memory.h" #include "util.h" +#include "files.h" #include "ignore-value.h" #include "virterror_internal.h" @@ -657,6 +658,8 @@ int virEventPollInit(void) virEventError(VIR_ERR_INTERNAL_ERROR, _("Unable to add handle %d to event loop"), eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]); return -1; } -- 1.7.1

3 2

Re: [libvirt] [libvirt-users] cannot perform tunnelled migration without using peer2peer flag
by Eric Blake 19 Jul '11

19 Jul '11

On 07/18/2011 04:11 AM, Osier Yang wrote: > 于 2011年07月18日 10:07, zhang xintao 写道: >> Dear All >> I try to migration a kvm guest os to another host failed >> server: ubuntu 11.04 server >> virsh:migrate --live --tunnelled vm1 qemu+ssh://192.168.10.3/system >> error:Requested operation is not valid:cannot perform >> tunnelled migration without using peer2peer flag > > The error tells you all, you need to use "--p2p". That said, why can't virsh be smarter, and automatically request the right underlying flags without making the user also type --p2p? Any problems with this patch? From db318d41c2a70189c21303c824aa4a862815874a Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake(a)redhat.com> Date: Mon, 18 Jul 2011 13:10:29 -0600 Subject: [PATCH] virsh: make migrate --tunnelled imply --p2p We can make the virsh migrate UI friendlier by supplying the missing bit automatically instead of erroring out when requesting --tunnelled without --p2p. * tools/virsh.c (doMigrate): Make --p2p optional when using --tunnelled. * tools/virsh.pod (migrate): Tweak wording accordingly. --- tools/virsh.c | 2 +- tools/virsh.pod | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 6c8032f..05aeb6c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4475,7 +4475,7 @@ doMigrate (void *opaque) if (vshCommandOptBool (cmd, "p2p")) flags |= VIR_MIGRATE_PEER2PEER; if (vshCommandOptBool (cmd, "tunnelled")) - flags |= VIR_MIGRATE_TUNNELLED; + flags |= VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER; if (vshCommandOptBool (cmd, "persistent")) flags |= VIR_MIGRATE_PERSIST_DEST; diff --git a/tools/virsh.pod b/tools/virsh.pod index e156232..e0adc31 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -534,14 +534,15 @@ Provide the maximum number of virtual CPUs supported for a guest VM on this connection. If provided, the I<type> parameter must be a valid type attribute for the <domain> element of XML. -=item B<migrate> [I<--live>] [I<--direct>] [I<--p2p> [I<--tunnelled>]] +=item B<migrate> [I<--live>] [I<--direct>] [I<--p2p>] [I<--tunnelled>] [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>] [I<--copy-storage-inc>] [I<--verbose>] I<domain-id> I<desturi> [I<migrateuri>] [I<dname>] [I<--timeout> B<seconds>] Migrate domain to another host. Add I<--live> for live migration; I<--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> -for tunnelled migration. I<--persistent> leaves the domain persistent on +for tunnelled migration (I<--tunnelled> implies I<--p2p>). I<--persistent> +leaves the domain persistent on destination host, I<--undefinesource> undefines the domain on the source host, and I<--suspend> leaves the domain paused on the destination host. I<--copy-storage-all> indicates migration with non-shared storage with full -- 1.7.4.4 -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

3 6

[libvirt] [PATCH] rpc: Make the dispatch generator handle 'void name(void)' style procedures
by Matthias Bolte 19 Jul '11

19 Jul '11

Currently there are no such procedures in the protocol, but programming errors like a missing _args or _ret suffix on the structs in the .x files can create such a situation by accident. Making the generator aware of this avoids bogus errors from the generator such as: Use of uninitialized value in exists at ./rpc/gendispatch.pl line 967. Reported by Michal Privoznik --- src/rpc/gendispatch.pl | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index e068b53..e6a4a1c 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -143,6 +143,20 @@ while (<PROTOCOL>) { $flags = $3; $ProcName = name_to_ProcName ($name); + if (!exists $calls{$name}) { + # that the argument and return value cases have not yet added + # this procedure to the calls hash means that it has no arguments + # and no return value. add it to the calls hash now because all + # procedures have to be listed in the calls hash + $calls{$name} = { + name => $name, + ProcName => $ProcName, + UC_NAME => uc $name, + args => "void", + ret => "void" + } + } + if ($opt_b or $opt_k) { if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(.*)\*\/\s*$/)) { die "invalid generator flags for ${procprefix}_PROC_${name}" -- 1.7.4.1

2 1

[libvirt] [PATCH] util: avoid fds leak on virEventPollInit
by ajia＠redhat.com 19 Jul '11

19 Jul '11

* src/util/event_poll.c: fix file descriptors leak on virEventPollInit. Detected in valgrind run: ==1254== ==1254== FILE DESCRIPTORS: 6 open at exit. ==1254== Open file descriptor 5: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== ==1254== Open file descriptor 4: ==1254== at 0x30736D9D47: pipe2 (syscall-template.S:82) ==1254== by 0x4DD6267: rpl_pipe2 (pipe2.c:61) ==1254== by 0x4C4C1C5: virEventPollInit (event_poll.c:648) ==1254== by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208) ==1254== by 0x42150C: main (virsh.c:13790) ==1254== * how to reproduce? % valgrind -v --track-fds=yes virsh list --- src/util/event_poll.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/util/event_poll.c b/src/util/event_poll.c index 285ba50..1e4ef96 100644 --- a/src/util/event_poll.c +++ b/src/util/event_poll.c @@ -639,6 +639,8 @@ static void virEventPollHandleWakeup(int watch ATTRIBUTE_UNUSED, int virEventPollInit(void) { + int ret = -1; + if (virMutexInit(&eventLoop.lock) < 0) { virReportSystemError(errno, "%s", _("Unable to initialize mutex")); @@ -648,7 +650,7 @@ int virEventPollInit(void) if (pipe2(eventLoop.wakeupfd, O_CLOEXEC | O_NONBLOCK) < 0) { virReportSystemError(errno, "%s", _("Unable to setup wakeup pipe")); - return -1; + goto cleanup; } if (virEventPollAddHandle(eventLoop.wakeupfd[0], @@ -657,10 +659,15 @@ int virEventPollInit(void) virEventError(VIR_ERR_INTERNAL_ERROR, _("Unable to add handle %d to event loop"), eventLoop.wakeupfd[0]); - return -1; + goto cleanup; } - return 0; + ret = 0; + +cleanup: + close(eventLoop.wakeupfd[0]); + close(eventLoop.wakeupfd[1]); + return ret; } static int virEventPollInterruptLocked(void) -- 1.7.1

5 8

[libvirt] [PATCH v3] util: avoid fds leak when virEventPollAddHandle fail
by ajia＠redhat.com 19 Jul '11

19 Jul '11

* src/util/event_poll.c: avoid file descriptors leak when virEventPollAddHandle fail on virEventPollInit function. --- src/util/event_poll.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/util/event_poll.c b/src/util/event_poll.c index 285ba50..e2ae3a6 100644 --- a/src/util/event_poll.c +++ b/src/util/event_poll.c @@ -36,6 +36,7 @@ #include "event_poll.h" #include "memory.h" #include "util.h" +#include "files.h" #include "ignore-value.h" #include "virterror_internal.h" @@ -657,6 +658,8 @@ int virEventPollInit(void) virEventError(VIR_ERR_INTERNAL_ERROR, _("Unable to add handle %d to event loop"), eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]); + VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]); return -1; } -- 1.7.1

1 0

[libvirt] [PATCH] interface: Check for interface (in-)activity on some operations
by Michal Privoznik 19 Jul '11

19 Jul '11

Right now it is possible to undefine an active interface, or destroy inactive. This patch add some checking to these operations to prevent this. Also fix test driver. --- src/interface/netcf_driver.c | 83 ++++++++++++++++++++++++++++++++++++------ src/test/test_driver.c | 5 +++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 855b5a3..bf590f3 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -119,6 +119,40 @@ static struct netcf_if *interfaceDriverGetNetcfIF(struct netcf *ncf, virInterfac return iface; } +/* + * interfaceObjIsActive: + * @iface - interface + * + * Caller MUST have driver locked to prevent race condition. + * Returns: + * -1 on error + * 0 on inactive interface + * 1 on active interface + */ +static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +interfaceObjIsActive(struct interface_driver *driver, + struct netcf_if *iface, + virInterfacePtr ifinfo) +{ + int ret = -1; + unsigned int flags = 0; + + if (ncf_if_status(iface, &flags) < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + interfaceReportError(netcf_to_vir_err(errcode), + _("failed to get status of interface %s: %s%s%s"), + ifinfo->name, errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } + + ret = flags & NETCF_IFACE_ACTIVE ? 1 : 0; + +cleanup: + return ret; +} + static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -438,6 +472,7 @@ static int interfaceUndefine(virInterfacePtr ifinfo) { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; int ret = -1; + int active; interfaceDriverLock(driver); @@ -447,6 +482,17 @@ static int interfaceUndefine(virInterfacePtr ifinfo) { goto cleanup; } + active = interfaceObjIsActive(driver, iface, ifinfo); + if (active < 0) { + /* helper already reported error */ + goto cleanup; + } else if (active) { + interfaceReportError(VIR_ERR_OPERATION_INVALID, + _("interface %s is active"), + ifinfo->name); + goto cleanup; + } + ret = ncf_if_undefine(iface); if (ret < 0) { const char *errmsg, *details; @@ -470,6 +516,7 @@ static int interfaceCreate(virInterfacePtr ifinfo, struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; int ret = -1; + int active; virCheckFlags(0, -1); @@ -481,6 +528,17 @@ static int interfaceCreate(virInterfacePtr ifinfo, goto cleanup; } + active = interfaceObjIsActive(driver, iface, ifinfo); + if (active < 0) { + /* helper already reported error */ + goto cleanup; + } else if (active) { + interfaceReportError(VIR_ERR_OPERATION_INVALID, + _("interface %s is already active"), + ifinfo->name); + goto cleanup; + } + ret = ncf_if_up(iface); if (ret < 0) { const char *errmsg, *details; @@ -504,6 +562,7 @@ static int interfaceDestroy(virInterfacePtr ifinfo, struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; int ret = -1; + int active; virCheckFlags(0, -1); @@ -515,6 +574,17 @@ static int interfaceDestroy(virInterfacePtr ifinfo, goto cleanup; } + active = interfaceObjIsActive(driver, iface, ifinfo); + if (active < 0) { + /* helper already reported error */ + goto cleanup; + } else if (!active) { + interfaceReportError(VIR_ERR_OPERATION_INVALID, + _("interface %s is not active"), + ifinfo->name); + goto cleanup; + } + ret = ncf_if_down(iface); if (ret < 0) { const char *errmsg, *details; @@ -536,7 +606,6 @@ static int interfaceIsActive(virInterfacePtr ifinfo) { struct interface_driver *driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; - unsigned int flags = 0; int ret = -1; interfaceDriverLock(driver); @@ -547,17 +616,7 @@ static int interfaceIsActive(virInterfacePtr ifinfo) goto cleanup; } - if (ncf_if_status(iface, &flags) < 0) { - const char *errmsg, *details; - int errcode = ncf_error(driver->netcf, &errmsg, &details); - interfaceReportError(netcf_to_vir_err(errcode), - _("failed to get status of interface %s: %s%s%s"), - ifinfo->name, errmsg, details ? " - " : "", - details ? details : ""); - goto cleanup; - } - - ret = flags & NETCF_IFACE_ACTIVE ? 1 : 0; + ret = interfaceObjIsActive(driver, iface, ifinfo); cleanup: ncf_if_free(iface); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f3fb320..d165586 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3648,6 +3648,11 @@ static int testInterfaceUndefine(virInterfacePtr iface) goto cleanup; } + if (privinterface->active != 0) { + testError(VIR_ERR_OPERATION_INVALID, NULL); + goto cleanup; + } + virInterfaceRemove(&privconn->ifaces, privinterface); ret = 0; -- 1.7.5.rc3

4 10

[libvirt] [PATCH v2 0/8] Introduce New API virDomainUndefineFlags
by Osier Yang 19 Jul '11

19 Jul '11

Per discussion on https://www.redhat.com/archives/libvir-list/2011-July/msg00556.html, This patch series introduce new API virDomainUndefineFlags, which only support flag VIR_DOMAIN_UNDEFINE_MANAGED_STATE now, might introduce more flags in future though. If the domain has a managed state file, then including VIR_DOMAIN_UNDEFINE_MANAGED_STATE in @flags will also remove that file, and omitting the flag will cause the undefine process to fail. This also changes the behavior of virDomainUndefine, but considering undefine a domain with managed state file left is unsafe, we need to do so. [PATCH v2 1/8] is small fix on rpc generator scripts, not related with the new API. [PATCH v2 1/8] rpc: Fix typos in rpc generator scripts [PATCH v2 2/8] undefine: Define the new API [PATCH v2 3/8] undefine: Wire up the remote protocol [PATCH v2 4/8] undefine: Implement internal API for qemu driver [PATCH v2 5/8] undefine: Implement internal API for libxl driver [PATCH v2 6/8] undefine: Implement undefineFlags in all other [PATCH v2 7/8] undefine: Extend virsh undefine to support the new [PATCH v2 8/8] virsh: Extend virsh dominfo to display if managed v1 - v2: * s/virDomainUndefineWithFlags/virDomainUndefineFlags/ * Update with other bunch of feedbacks from Eric * Changes on virsh dominfo to display if domain managed state exists Regards Osier

2 31

[libvirt] [PATCH] doc: Correct docs for iface commands
by Osier Yang 19 Jul '11

19 Jul '11

duplicate documents for "iface-name", lacks of document for "iface-mac", inconsistent option names with virsh help strings. --- tools/virsh.pod | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 377fac0..4f218d3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1081,20 +1081,24 @@ Returns the list of active host interfaces. If I<--all> is specified this will also include defined but inactive interfaces. If I<--inactive> is specified only the inactive ones will be listed. -=item B<iface-name> I<iface-MAC> +=item B<iface-name> I<interface> -Convert a host interface MAC to interface name, if the I<iface-MAC> is -unique among the host's interfaces. +Convert a host interface MAC to interface name, if the MAC address is unique +among the host's interfaces. -=item B<iface-name> I<iface-MAC> +I<interface> specifies the interface MAC address. + +=item B<iface-mac> I<interface> Convert a host interface name to MAC address. -=item B<iface-start> I<iface> +I<interface> specifies the interface name. + +=item B<iface-start> I<interface> Start a (previously defined) host interface, such as by running "if-up". -=item B<iface-undefine> I<iface> +=item B<iface-undefine> I<interface> Undefine the configuration for an inactive host interface. -- 1.7.6

2 2

[libvirt] [PATCHv4] libvirt: do not mix internal flags into public API
by Eric Blake 18 Jul '11

18 Jul '11

There were two API in driver.c that were silently masking flags bits prior to calling out to the drivers, and several others that were explicitly masking flags bits. This is not forward-compatible - if we ever have that many flags in the future, then talking to an old server that masks out the flags would be indistinguishable from talking to a new server that can honor the flag. In general, libvirt.c should forward _all_ flags on to drivers, and only the drivers should reject unknown flags. In the case of virDrvSecretGetValue, the solution is to separate the internal driver callback function to have two parameters instead of one, with only one parameter affected by the public API. In the case of virDomainGetXMLDesc, it turns out that no one was ever mixing VIR_DOMAIN_XML_INTERNAL_STATUS with the dumpxml path in the first place; that internal flag was only used in saving and restoring state files, which happened to be in functions internal to a single file, so there is no mixing of the internal flag with a public flags argument. Additionally, virDomainMemoryStats passed a flags argument over RPC, but not to the driver. * src/driver.h (VIR_DOMAIN_XML_FLAGS_MASK) (VIR_SECRET_GET_VALUE_FLAGS_MASK): Delete. (virDrvSecretGetValue): Separate out internal flags. (virDrvDomainMemoryStats): Provide missing flags argument. * src/driver.c (verify): Drop unused check. * src/conf/domain_conf.h (virDomainObjParseFile): Delete declaration. (virDomainXMLInternalFlags): Move... * src/conf/domain_conf.c: ...here. Delete redundant include. (virDomainObjParseFile): Make static. * src/libvirt.c (virDomainGetXMLDesc, virSecretGetValue): Update clients. (virDomainMemoryPeek, virInterfaceGetXMLDesc) (virDomainMemoryStats, virDomainBlockPeek, virNetworkGetXMLDesc) (virStoragePoolGetXMLDesc, virStorageVolGetXMLDesc) (virNodeNumOfDevices, virNodeListDevices, virNWFilterGetXMLDesc): Don't mask unknown flags. * src/interface/netcf_driver.c (interfaceGetXMLDesc): Reject unknown flags. * src/secret/secret_driver.c (secretGetValue): Update clients. * src/remote/remote_driver.c (remoteSecretGetValue) (remoteDomainMemoryStats): Likewise. * src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase): Likewise. * src/qemu/qemu_driver.c (qemudDomainMemoryStats): Likewise. * daemon/remote.c (remoteDispatchDomainMemoryStats): Likewise. --- v4: fix even more public libvirt API flag masking v3 at https://www.redhat.com/archives/libvir-list/2011-July/msg00894.html daemon/remote.c | 2 +- src/conf/domain_conf.c | 16 +++++++++---- src/conf/domain_conf.h | 10 -------- src/driver.c | 4 --- src/driver.h | 16 +++++-------- src/interface/netcf_driver.c | 2 + src/libvirt.c | 51 +++++------------------------------------ src/qemu/qemu_driver.c | 5 +++- src/qemu/qemu_process.c | 2 +- src/remote/remote_driver.c | 13 ++++++++-- src/secret/secret_driver.c | 7 ++++- 11 files changed, 47 insertions(+), 81 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index e93a0dc..daad39d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -810,7 +810,7 @@ remoteDispatchDomainMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - nr_stats = virDomainMemoryStats(dom, stats, args->maxStats, 0); + nr_stats = virDomainMemoryStats(dom, stats, args->maxStats, args->flags); if (nr_stats < 0) goto cleanup; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8c3e44e..3c3ab39 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -48,7 +48,6 @@ #include "storage_file.h" #include "files.h" #include "bitmap.h" -#include "verify.h" #include "count-one-bits.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -57,6 +56,12 @@ * verify that it doesn't overflow an unsigned int when shifting */ verify(VIR_DOMAIN_VIRT_LAST <= 32); +/* Private flag used internally by virDomainSaveStatus and + * virDomainObjParseFile. */ +typedef enum { + VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain status information */ +} virDomainXMLInternalFlags; + VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST, "custom-argv", "custom-monitor", @@ -6995,10 +7000,11 @@ cleanup: } -virDomainObjPtr virDomainObjParseFile(virCapsPtr caps, - const char *filename, - unsigned int expectedVirtTypes, - unsigned int flags) +static virDomainObjPtr +virDomainObjParseFile(virCapsPtr caps, + const char *filename, + unsigned int expectedVirtTypes, + unsigned int flags) { xmlDocPtr xml; virDomainObjPtr obj = NULL; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 172d3c2..7a3d72b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -41,11 +41,6 @@ # include "macvtap.h" # include "sysinfo.h" -/* Private component of virDomainXMLFlags */ -typedef enum { - VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain status information */ -} virDomainXMLInternalFlags; - /* Different types of hypervisor */ /* NB: Keep in sync with virDomainVirtTypeToString impl */ enum virDomainVirtType { @@ -1417,11 +1412,6 @@ virDomainDefPtr virDomainDefParseNode(virCapsPtr caps, unsigned int expectedVirtTypes, unsigned int flags); -virDomainObjPtr virDomainObjParseFile(virCapsPtr caps, - const char *filename, - unsigned int expectedVirtTypes, - unsigned int flags); - bool virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefPtr dst); diff --git a/src/driver.c b/src/driver.c index 579c2b3..5034277 100644 --- a/src/driver.c +++ b/src/driver.c @@ -32,10 +32,6 @@ #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver" -/* Make sure ... INTERNAL_CALL cannot be set by the caller */ -verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & - VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); - #ifdef WITH_DRIVER_MODULES /* XXX re-implment this for other OS, or use libtools helper lib ? */ diff --git a/src/driver.h b/src/driver.h index 70ea4c2..9d0d3de 100644 --- a/src/driver.h +++ b/src/driver.h @@ -191,7 +191,7 @@ typedef char * unsigned int flags); typedef char * (*virDrvDomainGetXMLDesc) (virDomainPtr dom, - unsigned int flags); + unsigned int flags); typedef char * (*virDrvConnectDomainXMLFromNative) (virConnectPtr conn, const char *nativeFormat, @@ -331,7 +331,8 @@ typedef int (*virDrvDomainMemoryStats) (virDomainPtr domain, struct _virDomainMemoryStat *stats, - unsigned int nr_stats); + unsigned int nr_stats, + unsigned int flags); typedef int (*virDrvDomainBlockPeek) @@ -1229,16 +1230,10 @@ struct _virDeviceMonitor { virDrvNodeDeviceDestroy deviceDestroy; }; -/* bits 16 and above of virDomainXMLFlags are for internal use */ -# define VIR_DOMAIN_XML_FLAGS_MASK 0xffff - -/* Bits 16 and above of virSecretGetValue flags are for internal use */ -# define VIR_SECRET_GET_VALUE_FLAGS_MASK 0xffff - enum { /* This getValue call is inside libvirt, override the "private" flag. This flag cannot be set by outside callers. */ - VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 16 + VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 0, }; typedef virSecretPtr @@ -1263,7 +1258,8 @@ typedef int typedef unsigned char * (*virDrvSecretGetValue) (virSecretPtr secret, size_t *value_size, - unsigned int flags); + unsigned int flags, + unsigned int internalFlags); typedef int (*virDrvSecretUndefine) (virSecretPtr secret); typedef int diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 855b5a3..2f322b4 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -344,6 +344,8 @@ static char *interfaceGetXMLDesc(virInterfacePtr ifinfo, virInterfaceDefPtr ifacedef = NULL; char *ret = NULL; + virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL); + interfaceDriverLock(driver); iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo); diff --git a/src/libvirt.c b/src/libvirt.c index 7c68bd8..4de718d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3381,8 +3381,6 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) goto error; } - flags &= VIR_DOMAIN_XML_FLAGS_MASK; - if (conn->driver->domainGetXMLDesc) { char *ret; ret = conn->driver->domainGetXMLDesc(domain, flags); @@ -6031,11 +6029,6 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, virDispatchError(NULL); return -1; } - if (flags != 0) { - virLibDomainError(VIR_ERR_INVALID_ARG, - _("flags must be zero")); - goto error; - } if (!stats || nr_stats == 0) return 0; @@ -6045,7 +6038,8 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, conn = dom->conn; if (conn->driver->domainMemoryStats) { - nr_stats_ret = conn->driver->domainMemoryStats (dom, stats, nr_stats); + nr_stats_ret = conn->driver->domainMemoryStats (dom, stats, nr_stats, + flags); if (nr_stats_ret == -1) goto error; return nr_stats_ret; @@ -6130,12 +6124,6 @@ virDomainBlockPeek (virDomainPtr dom, goto error; } - if (flags != 0) { - virLibDomainError(VIR_ERR_INVALID_ARG, - _("flags must be zero")); - goto error; - } - /* Allow size == 0 as an access test. */ if (size > 0 && !buffer) { virLibDomainError(VIR_ERR_INVALID_ARG, @@ -6239,9 +6227,10 @@ virDomainMemoryPeek (virDomainPtr dom, * because of incompatible licensing. */ - if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) { + /* Exactly one of these two flags must be set. */ + if (!(flags & VIR_MEMORY_VIRTUAL) == !(flags & VIR_MEMORY_PHYSICAL)) { virLibDomainError(VIR_ERR_INVALID_ARG, - _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL")); + _("flags parameter must include VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL")); goto error; } @@ -8465,10 +8454,6 @@ virNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) virDispatchError(NULL); return NULL; } - if (flags != 0) { - virLibNetworkError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } conn = network->conn; @@ -8978,10 +8963,6 @@ virInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags) virDispatchError(NULL); return NULL; } - if ((flags & ~VIR_INTERFACE_XML_INACTIVE) != 0) { - virLibInterfaceError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } conn = iface->conn; @@ -10451,10 +10432,6 @@ virStoragePoolGetXMLDesc(virStoragePoolPtr pool, virDispatchError(NULL); return NULL; } - if (flags != 0) { - virLibStoragePoolError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } conn = pool->conn; @@ -11349,10 +11326,6 @@ virStorageVolGetXMLDesc(virStorageVolPtr vol, virDispatchError(NULL); return NULL; } - if (flags != 0) { - virLibStorageVolError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } conn = vol->conn; @@ -11441,10 +11414,6 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags) virDispatchError(NULL); return -1; } - if (flags != 0) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } if (conn->deviceMonitor && conn->deviceMonitor->numOfDevices) { int ret; @@ -11493,7 +11462,7 @@ virNodeListDevices(virConnectPtr conn, virDispatchError(NULL); return -1; } - if ((flags != 0) || (names == NULL) || (maxnames < 0)) { + if ((names == NULL) || (maxnames < 0)) { virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } @@ -12699,12 +12668,10 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags) goto error; } - flags &= VIR_SECRET_GET_VALUE_FLAGS_MASK; - if (conn->secretDriver != NULL && conn->secretDriver->getValue != NULL) { unsigned char *ret; - ret = conn->secretDriver->getValue(secret, value_size, flags); + ret = conn->secretDriver->getValue(secret, value_size, flags, 0); if (ret == NULL) goto error; return ret; @@ -14233,10 +14200,6 @@ virNWFilterGetXMLDesc(virNWFilterPtr nwfilter, unsigned int flags) virDispatchError(NULL); return NULL; } - if (flags != 0) { - virLibNWFilterError(VIR_ERR_INVALID_ARG, __FUNCTION__); - goto error; - } conn = nwfilter->conn; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8d146aa..1a3fbfb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6155,12 +6155,15 @@ qemudDomainInterfaceStats (virDomainPtr dom, static int qemudDomainMemoryStats (virDomainPtr dom, struct _virDomainMemoryStat *stats, - unsigned int nr_stats) + unsigned int nr_stats, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; unsigned int ret = -1; + virCheckFlags(0, -1); + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d0085e0..448b06e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -276,7 +276,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn, enc->secrets[0]->uuid); if (secret == NULL) goto cleanup; - data = conn->secretDriver->getValue(secret, &size, + data = conn->secretDriver->getValue(secret, &size, 0, VIR_SECRET_GET_VALUE_INTERNAL_CALL); virUnrefSecret(secret); if (data == NULL) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 2d5dc15..c2f8bbd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1850,7 +1850,8 @@ done: static int remoteDomainMemoryStats (virDomainPtr domain, struct _virDomainMemoryStat *stats, - unsigned int nr_stats) + unsigned int nr_stats, + unsigned int flags) { int rv = -1; remote_domain_memory_stats_args args; @@ -1868,7 +1869,7 @@ remoteDomainMemoryStats (virDomainPtr domain, goto done; } args.maxStats = nr_stats; - args.flags = 0; + args.flags = flags; memset (&ret, 0, sizeof ret); if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_MEMORY_STATS, @@ -3173,7 +3174,7 @@ remoteSecretClose (virConnectPtr conn) static unsigned char * remoteSecretGetValue (virSecretPtr secret, size_t *value_size, - unsigned int flags) + unsigned int flags, unsigned int internalFlags) { unsigned char *rv = NULL; remote_secret_get_value_args args; @@ -3182,6 +3183,12 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size, remoteDriverLock (priv); + /* internalFlags intentionally do not go over the wire */ + if (internalFlags) { + remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no internalFlags support")); + goto done; + } + make_nonnull_secret (&args.secret, secret); args.flags = flags; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index c45ba51..02cdbb9 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -873,12 +873,15 @@ cleanup: } static unsigned char * -secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags) +secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags, + unsigned int internalFlags) { virSecretDriverStatePtr driver = obj->conn->secretPrivateData; unsigned char *ret = NULL; virSecretEntryPtr secret; + virCheckFlags(0, NULL); + secretDriverLock(driver); secret = secretFindByUUID(driver, obj->uuid); @@ -898,7 +901,7 @@ secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags) goto cleanup; } - if ((flags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 && + if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 && secret->def->private) { virSecretReportError(VIR_ERR_OPERATION_DENIED, "%s", _("secret is private")); -- 1.7.4.4

2 6