- Devel - libvirt List Archives

[libvirt] [PATCH] Improve error reporting in virsh
by john.levon＠sun.com 09 Feb '09

09 Feb '09

# HG changeset patch # User john.levon(a)sun.com # Date 1233870902 28800 # Node ID fe7d1aae6184476568fcd3da379207375d8a05a6 # Parent 9fecb1254ee1dad6efeedc50239852104a6f40e2 Improve error reporting in virsh Rather than verbosely printing every error, save the last error and report that only if the entire command fails. Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/virsh.c b/src/virsh.c --- a/src/virsh.c +++ b/src/virsh.c @@ -93,22 +93,6 @@ typedef enum { } vshErrorLevel; /* - * The error handler for virsh - */ -static void -virshErrorHandler(void *unused, virErrorPtr error) -{ - if ((unused != NULL) || (error == NULL)) - return; - - /* Suppress the VIR_ERR_NO_XEN error which fails as non-root */ - if ((error->code == VIR_ERR_NO_XEN) || (error->code == VIR_ERR_OK)) - return; - - virDefaultErrorFunc(error); -} - -/* * virsh command line grammar: * * command_line = <command>\n | <command>; <command>; ... @@ -319,6 +303,46 @@ static int namesorter(const void *a, con const char **sb = (const char**)b; return strcasecmp(*sa, *sb); +} + +static virErrorPtr last_error; + +/* + * Quieten libvirt until we're done with the command. + */ +static void +virshErrorHandler(void *unused, virErrorPtr error) +{ + virFreeError(last_error); + last_error = virSaveLastError(); + if (getenv("VIRSH_DEBUG") != NULL) + virDefaultErrorFunc(error); +} + +/* + * Report an error when a command finishes. This is better than before + * (when correct operation would report errors), but it has some + * problems: we lose the smarter formatting of virDefaultErrorFunc(), + * and it can become harder to debug problems, if errors get reported + * twice during one command. This case shouldn't really happen anyway, + * and it's IMHO a bug that libvirt does that sometimes. + */ +static void +virshReportError(vshControl *ctl) +{ + if (last_error == NULL) + return; + + if (last_error->code == VIR_ERR_OK) { + vshError(ctl, FALSE, "%s", _("unknown error")); + goto out; + } + + vshError(ctl, FALSE, "%s", last_error->message); + +out: + virFreeError(last_error); + last_error = NULL; } @@ -6102,6 +6126,9 @@ vshCommandRun(vshControl *ctl, const vsh if (ctl->timing) GETTIMEOFDAY(&after); + + if (ret == FALSE) + virshReportError(ctl); if (STREQ(cmd->def->name, "quit")) /* hack ... */ return ret;

2 1

[libvirt] [PATCH] Introduce virSaveLastError(), virFreeError()
by john.levon＠sun.com 09 Feb '09

09 Feb '09

# HG changeset patch # User john.levon(a)sun.com # Date 1233870901 28800 # Node ID 2c12bdf95e5e5f7d00b606df679364463555d76e # Parent 059167552cb467c107870c3b739438ca107116a8 Introduce virSaveLastError(), virFreeError() Allow applications to store a copy of the last error reported. Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -172,8 +172,10 @@ typedef void (*virErrorFunc) (void *user */ virErrorPtr virGetLastError (void); +virErrorPtr virSaveLastError (void); void virResetLastError (void); void virResetError (virErrorPtr err); +void virFreeError (virErrorPtr err); virErrorPtr virConnGetLastError (virConnectPtr conn); void virConnResetLastError (virConnectPtr conn); diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -247,4 +247,10 @@ LIBVIRT_0.6.0 { } LIBVIRT_0.5.0; +LIBVIRT_0.6.1 { + global: + virFreeError; + virSaveLastError; +} LIBVIRT_0.6.0; + # .... define new API here using predicted next version number .... diff --git a/src/virterror.c b/src/virterror.c --- a/src/virterror.c +++ b/src/virterror.c @@ -286,6 +286,27 @@ virCopyLastError(virErrorPtr to) } /** + * virSaveLastError: + * + * Save the last error into a new error object. + * + * Returns a pointer to the copied error or NULL if allocation failed. + * It is the caller's responsibility to free the error with + * virFreeError(). + */ +virErrorPtr +virSaveLastError(void) +{ + virErrorPtr to; + + if (VIR_ALLOC(to) < 0) + return NULL; + + virCopyLastError(to); + return to; +} + +/** * virResetError: * @err: pointer to the virError to clean up * @@ -303,6 +324,18 @@ virResetError(virErrorPtr err) memset(err, 0, sizeof(virError)); } +/** + * virFreeError: + * @err: error to free + * + * Resets and frees the given error. + */ +void +virFreeError(virErrorPtr err) +{ + virResetError(err); + VIR_FREE(err); +} /** * virResetLastError:

2 1

[libvirt] Leaky virsh?
by John Levon 09 Feb '09

09 Feb '09

Running virsh with the remote driver (as it always is on Solaris), it leaks pretty heavily: thewhip:~ # yes "start fewfewf" | virsh >/dev/null 2>&1 & [1] 6388 # prstat -c -p 6388 PID USERNAME SIZE RSS STATE PRI NICE TIME CPU PROCESS/NLWP 6388 root 10M 6788K sleep 59 0 0:00:02 0.2% virsh/1 Total: 1 processes, 1 lwps, load averages: 0.32, 0.12, 0.06 PID USERNAME SIZE RSS STATE PRI NICE TIME CPU PROCESS/NLWP 6388 root 11M 7104K sleep 59 0 0:00:02 0.3% virsh/1 Total: 1 processes, 1 lwps, load averages: 0.34, 0.12, 0.06 PID USERNAME SIZE RSS STATE PRI NICE TIME CPU PROCESS/NLWP 6388 root 11M 7160K sleep 59 0 0:00:02 0.3% virsh/1 Total: 1 processes, 1 lwps, load averages: 0.36, 0.13, 0.07 Can you reproduce on Linux anyone? regards john

3 3

[libvirt] [PATCH/RFC]: don't ignore errors to save the domain status file
by Guido Günther 07 Feb '09

07 Feb '09

Hi, we currently don't report errors to save qemu's domain status file back to the caller. That was o.k. as long as the code was there for testing but now that the XML is being picked up on daemon restart we must handle these. The attached patch does that, although I'm not confident that it's enough. While we return error on device attach/unattach the device actually got attached/unattached already. Same is true for suspend/resume. A better solution would be to write out the new domain status into a temp file and simply call rename(2) on it when the attach/unattach succeeds and discard it if it fails. This would minimize the possible error conditions like disk full, missing permissions to write into that directory etc. Does this sound reasonable? O.k. to apply the attached version for now? Cheers, -- Guido

3 4

[libvirt] [PATCH] turn off strict-aliasing warnings in two files
by Jim Meyering 06 Feb '09

06 Feb '09

Without these changes and with gcc-4.4, I'd get 4 warnings (2 in each file) about strict-aliasing violations. -Wstrict-aliasing is worth keeping in general, so I have disabled it only for the two offending files. The #if avoids the pragma on versions of gcc that don't recognize it. >From 7d7f8e5dd9754d2bbca1a0ec2f3a47d4eae9df30 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 6 Feb 2009 11:45:39 +0100 Subject: [PATCH] turn off strict-aliasing warnings in two files * qemud/qemud.c: Disable gcc's strict-aliasing warnings in this file. * src/bridge.c: Likewise. --- qemud/qemud.c | 5 +++++ src/bridge.c | 5 +++++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index effb336..c3521e3 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -21,6 +21,11 @@ * Author: Daniel P. Berrange <berrange(a)redhat.com> */ +/* Tell gcc not to warn about aliasing (e.g., sockaddr_in) in this file. */ +#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__ +# pragma GCC diagnostic ignored "-Wstrict-aliasing" +#endif + #include <config.h> #include <sys/types.h> diff --git a/src/bridge.c b/src/bridge.c index 8e577dd..62fa711 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -19,6 +19,11 @@ * Mark McLoughlin <markmc(a)redhat.com> */ +/* Tell gcc not to warn about aliasing (e.g., sockaddr_in) in this file. */ +#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__ +# pragma GCC diagnostic ignored "-Wstrict-aliasing" +#endif + #include <config.h> #if defined(WITH_BRIDGE) -- 1.6.1.2.511.gc5d3f

2 3

[libvirt] I have been looking over James Morris patches for svirt.
by Daniel J Walsh 06 Feb '09

06 Feb '09

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 What is the process to get them into libvirt? I have begun to look at the second componant of the libvirt change. James patch, allows libvirt to read the SELinux context out of the xml database and execute qemu with the context. The second componant is to pass the context of the image(s) and allow libvirt to not only set the image, but also update the default labels on disk, so a relabel will not change the context. I have some patches to do this, but want to make sure the original patches are acceptable? The last changes and perhaps the most difficult is figuring out how to get the labels into the XML database in the first place. I guess virt_manager will somehow figure out what the labels should be and assign them. Dan -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmMeZEACgkQrlYvE4MpobMZcwCePmE1aF3zJURS2up7ERoxNU4V Ho8AoIwBMN7/f4zBCIDTpbnLg13FfPOG =rHH5 -----END PGP SIGNATURE-----

2 1

[libvirt] FreeNodeMemory
by Gihan Munasinghe 06 Feb '09

06 Feb '09

Hi Is there a specific reason for not having a "free_memory" with in the "virNodeInfo" structure. I can see there is "virNodeGetFreeMemory(connection)" which currently not implemented for XEN.. I can add the patch to do this.. But I would like to clarify why we have not add "free_memory" in to the virNodeInfo structure.. Thanks -- Gihan Munasinghe R&D Team Leader XCalibre Communications Ltd. www.flexiscale.com

4 9

[libvirt] PATCH: Fix multiple bugs in RPC handling
by Daniel P. Berrange 06 Feb '09

06 Feb '09

A number of bugs conspired together to cause some nasty problems when a QEMU vm failed to start - vm->monitor was not initialized to -1, so when a VM failed to start the vm->monitor was just '0', and thus we closed FD 0 (libvirtd's stdin) - The next client to connect got FD 0 as its socket - The first bug struck again, causing the client to be closed even though libvirt thought it was still open - libvirtd now polle on FD=0, which gave back POLLNVAL because it was closed - event.c was not looking for POLLNVAL so it span 100% cpu when this happened, instead of invoking the callback with an error code - virsh was not cleaning up the priv->watiDispatch call upon I/O errors, so virsh then hung when doing virConenctClose This patch does 3 things - Treats POLLNVAL as VIR_EVENT_HANDLE_ERROR, so the callback gets to see the error & de-registers the client from the event loop - Add the missing initialization of vm->monitor - Fix remote_internal.c handling of I/O errors Daniel Index: qemud/event.c =================================================================== RCS file: /data/cvs/libvirt/qemud/event.c,v retrieving revision 1.17 diff -u -p -u -p -r1.17 event.c --- qemud/event.c 22 Dec 2008 12:55:47 -0000 1.17 +++ qemud/event.c 5 Feb 2009 17:12:51 -0000 @@ -653,6 +653,8 @@ virPollEventToEventHandleType(int events ret |= VIR_EVENT_HANDLE_WRITABLE; if(events & POLLERR) ret |= VIR_EVENT_HANDLE_ERROR; + if(events & POLLNVAL) /* Treat NVAL as error, since libvirt doesn't distinguish */ + ret |= VIR_EVENT_HANDLE_ERROR; if(events & POLLHUP) ret |= VIR_EVENT_HANDLE_HANGUP; return ret; Index: src/domain_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/domain_conf.c,v retrieving revision 1.64 diff -u -p -u -p -r1.64 domain_conf.c --- src/domain_conf.c 30 Jan 2009 21:52:22 -0000 1.64 +++ src/domain_conf.c 5 Feb 2009 17:12:51 -0000 @@ -504,6 +504,7 @@ virDomainObjPtr virDomainAssignDef(virCo domain->state = VIR_DOMAIN_SHUTOFF; domain->def = def; domain->monitor_watch = -1; + domain->monitor = -1; if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) { virReportOOMError(conn); Index: src/remote_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/remote_internal.c,v retrieving revision 1.136 diff -u -p -u -p -r1.136 remote_internal.c --- src/remote_internal.c 3 Feb 2009 13:08:07 -0000 1.136 +++ src/remote_internal.c 5 Feb 2009 17:12:52 -0000 @@ -6192,17 +6192,17 @@ processCalls(virConnectPtr conn, continue; virReportSystemError(in_open ? NULL : conn, errno, "%s", _("poll on socket failed")); - return -1; + goto error; } if (fds[0].revents & POLLOUT) { if (processCallSend(conn, priv, in_open) < 0) - return -1; + goto error; } if (fds[0].revents & POLLIN) { if (processCallRecv(conn, priv, in_open) < 0) - return -1; + goto error; } /* Iterate through waiting threads and if @@ -6253,9 +6253,21 @@ processCalls(virConnectPtr conn, if (fds[0].revents & (POLLHUP | POLLERR)) { errorf(in_open ? NULL : conn, VIR_ERR_INTERNAL_ERROR, "%s", _("received hangup / error event on socket")); - return -1; + goto error; } } + + +error: + priv->waitDispatch = thiscall->next; + DEBUG("Giving up the buck due to I/O error %d %p %p", thiscall->proc_nr, thiscall, priv->waitDispatch); + /* See if someone else is still waiting + * and if so, then pass the buck ! */ + if (priv->waitDispatch) { + DEBUG("Passing the buck to %d %p", priv->waitDispatch->proc_nr, priv->waitDispatch); + virCondSignal(&priv->waitDispatch->cond); + } + return -1; } /* -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] storage error
by Farkas Levente 06 Feb '09

06 Feb '09

hi, first of all i don't know which is the right list for this. since it's created by virt-manager, but the error says libvirtError. anyway another error during a new guest install. ------------------------------------------- [Fri, 06 Feb 2009 10:54:14 virt-manager 32039] DEBUG (error:76) Uncaught Error: Uncaught error validating input: cannot open volume '/var/lib/libvirt/images/fedora-i386.img': No such file or directory : Trac eback (most recent call last): File "/usr/share/virt-manager/virtManager/create.py", line 301, in forward if(self.validate(notebook.get_current_page()) != True): File "/usr/share/virt-manager/virtManager/create.py", line 1004, in validate conn=self._guest.conn) File "/usr/lib/python2.4/site-packages/virtinst/VirtualDisk.py", line 166, in __init__ self.__validate_params() File "/usr/lib/python2.4/site-packages/virtinst/VirtualDisk.py", line 492, in __validate_params self.__set_dev_type() File "/usr/lib/python2.4/site-packages/virtinst/VirtualDisk.py", line 290, in __set_dev_type t = self.vol_object.info()[0] File "/usr/lib64/python2.4/site-packages/libvirt.py", line 947, in info if ret is None: raise libvirtError ('virStorageVolGetInfo() failed', vol=self) libvirtError: cannot open volume '/var/lib/libvirt/images/fedora-i386.img': No such file or directory ------------------------------------------- the current situation i'm not able to install any new guests:-( -- Levente "Si vis pacem para bellum!"

1 0

[libvirt] RFC: configuring host interfaces with libvirt
by David Lutterkort 06 Feb '09

06 Feb '09

For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. Below is a high-level proposal of how that could be done. Please comment copiously ;) 1. XML Format ============= The first question is how the user would describe the host interfaces they want. Below are some sketches of what an XML representation of the various kinds of interfaces would look like. This covers the minimal amount of settings for these to be useful, though I am sure we'll need to add more over time. <interface device="eth0" onboot="yes"> <hwaddr>00:19:d2:9f:88:96</hwaddr> <dhcp peerdns="yes"/> </interface> <interface device="eth1" onboot="yes"> <hwaddr>00:19:d2:9f:88:97</hwaddr> <static ipaddr="192.168.0.5" gateway="192.168.0.1" netmask="255.255.255.0"/> </interface> <interface device="eth2" onboot="yes"> <hwaddr>00:19:d2:9f:88:98</hwaddr> </interface> <bond name="bond00" onboot="yes" mode="active-backup"> <slave device="eth0" primary="yes"/> <slave device="eth1"/> </bond> <bridge name="br0" stp="off" onboot="yes"> <member device="eth2"/> <dhcp peerdns="yes"/> </bridge> <vlan device="eth0" tag="42" reorder_hdr="yes"/> All of these should also allow a <uuid> element for specifying a uuid; I omitted that for brevity. 2. API Changes ============== There are two options for dealing with network interfaces: (1) use the existing virNetwork* calls or (2) add completely new API calls. Repurposing existing virNetwork* calls -------------------------------------- The existing calls map well to the operations we need for managing interfaces, with a few exceptions: - virNetworkGetAutostart/SetAutostart: depending on how we implement all this (see below), 'autostart' might actually mean 'on boot', not 'when libvirtd starts' - virNetworkGetBridgeName doesn't make sense for interfaces, and should return NULL for interfaces We'll probably also end up adding some functions to query details about an interface, in particular, a call to see what kind of network/interface a virNetworkPtr represents Add completely new virInterface* calls -------------------------------------- This would add roughly the same API calls as the virNetwork* calls, i.e. we'd have something like typedef struct virInterface *virInterfacePtr; int virInterfaceCreate(virInterfacePtr); virInterfacePtr virInterfaceCreateXML(..); ... plus some calls to extract information from a virInterfacePtr The second option seems cleaner to me and easier to implement, and avoids any subtle changes in the behavior of existing API, though I don't like that we'll be adding another 20 or so calls to libvirt's public API, with attendant churn both in the drivers and in virsh. 3. Implementation ================= Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) Option (1) saves us from replicating every bit of network setup functionality that those scripts already have - besides configuring the interface, we also might have to setup routes, run dhclient etc. Option (2) would require far fewer backend implementations than (1) - we should be able to get away with one implementation for Linux, rather than one for Fedora/RHEL, one for Debian, one for SuSe, three for gentoo etc. If we want 'autostart' for an interface to mean 'bring up the interface as soon as the system boots', we are pretty much forced to go with option (1). All in all, option (1) seems more attractive, since it should save us from dealing with a lot of low-level details of network setup, and the distro scripts should be much better integrated with the rest of the system than what we come up with for libvirt. 4. Misc issues ============== * Should interfaces have labels/roles ('data-interface') to help admins make sense of the current config ? * Do we expect interfaces to be in a specific state before we create them or do we just tear them down and reconfigure them no matter what ? * Are there crucial config options that are not covered by the sketches above (e.g., setting an explicit MTU) ? Are there things in the XML sketches above that will be impossible to implement on some OS ? * Should this even be done as part of libvirt ? It seems like a very generic network config tool, and libvirt merely the conduit to exposing this through an API, most importantly, a remotable API. David

9 36