RE: [libvirt] invoking interface.script - missing interface name of domain
Hi Daniel, within the latest CVS checkout, the problem below is not fixed. It there an open bug (to track) in redhat bugzilla for it ? If not, should I create one ? regards Danny On Tue, Jan 20, 2009 at 04:44:22PM +0100, Daniel Schwager wrote: QEMU is responsible for passing arguments to the interface scripts, so its out of libvirt's control. QEMU passes a single paramter to the script, which is the name of the TAP device interface being added. If you don't give a target dev='tap3' libvirt generates a TAP device name vnet. If I do not specify the target-element inside of interface, your docu told me The guest will have a tun device created with a name of vnetN, which can also be overridden with the target element. (http://www.libvirt.org/formatdomain.html#elementsNICS) That is a bug I'm afraid - it *should* pass the real auto-generated interface name but for some reason it isn't. I see this is one of the XML configs we don't have tested in our xml - ARGV conversion test. ... Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: style question (was Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.)
On Thu, Jan 29, 2009 at 10:48:36PM -0500, Dave Allan wrote: Jim Meyering wrote: Finally, I moved a couple variable declarations down (C99-style) to their points of first use. I take it that the C++/C99 is the recommended style for all libvirt code? I generally haven't coded this way in the past (in fact I usually compile with -Wdeclaration-after-statement), and not seeing anything in HACKING about it, I continued to use the traditional style, but if this way is what everyone's converging on, I'll change to it. My preference is for declarations immediately after the start of a block. eg, at start of a function, or immediately at start of a nested if / while. I don't like have declarations scattered elsewhere in the functions because it means you end up having to scan the entire function body to find where variables are declared, instead of just looking at the start of the code block you are in. The vast majority of libvirt code works this way, so I'd rather stick with the traditional style and cleanup the few places not in compliance. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: style question
Dave Allan dal...@redhat.com wrote: Jim Meyering wrote: Finally, I moved a couple variable declarations down (C99-style) to their points of first use. I take it that the C++/C99 is the recommended style for all libvirt code? I generally haven't coded this way in the past (in fact I usually compile with -Wdeclaration-after-statement), and not seeing anything in HACKING about it, I continued to use the traditional style, but if this way is what everyone's converging on, I'll change to it. Until a couple years ago, the de-facto standard in libvirt (like many projects) was to use C89-style all-declarations-at-opening-brace, but since no reasonable portability target lacks a C99 compiler, we are slowly modernizing with the addition of new code. If you would like a long list of reasons for using this aspect of C99, let me know. Last night I was peeved to have to deal with a pointless conflict because a declaration-addition evoked a conflict. Guess why? Because adding it at the beginning of the function conflicted with another change that had also appended to that function-global list, but for a different variable name. Ironically, both were for uses that were much farther down and very local, so putting the respective declarations near their uses would have made the patches and code smaller and more readable, in addition to avoiding my conflict. I propose mention in HACKING that it is permitted (encouraged, even) to use C99 decls-after-stmt, but that you won't get dinged (yet) for using the old style. IME, readability and maintainability are important enough to require minimum distance between decl and first use of each variable, but others may have different habits and resist the change. Let's see. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] kvm, sync guest disk before saving ?
On Fri, Jan 30, 2009 at 12:44:14AM +0100, Daniel Schwager wrote: Hi, is there a way to get a consistent disk image of a domain running qemu-kvm before saving it ? I used xm sysrq id s with xen-hypervisor - but what's the way with kvm ? Or could I install drivers into the guest helping to solve this problem ? 'xm sysrq s' is not giving you a consistent disk image, because between the time you issue that command, and the time you save the disk/vm more I/O could have been issued. Ultimately if you want consistent disk images, you either need to stop the VM, and clne the disk. Or save the VM state to a file, and then clone the disk. Or use a 'vm snapshot' capability which does those two operations in one (which potentially allows for live snapshotting, which a 2-step process would not). We don't yet support the latter, but you can do save/restore of the VM state to a file, and snapshot the disk at that time. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] bug: libvirt hang while restore a domain
On Fri, Jan 30, 2009 at 01:14:57AM +0100, Daniel Schwager wrote: ** But the second time, the command hang: [r...@xen03 srv]# virsh restore /srv/save And libvirt daemon runs with 100% CPU load .. This is not good .. I know about this problem am working on a fix - the refactored startup code was not taking account of some failure scenarios, and ending up in an infinite loop Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: Avoid crashing valgrind in LXC driver
The LXC driver makes use of new clone flags for creating containers. It
creates a dummy container which immediately exits in order to test for
availabilty of this feature in the kernel. Unfortunately valgrind has
no knowledge of these new clone flags, gets very very unhappy and then
reports bogus memory leaks, bogus illegal instructions, and then often
SEGV's itself. Not cool. While obviously valgrind needs fixing, I looked
at its code, and it doesn't seem easy, so this patch adds a quick check
for a LD_PRELOAD environemnt variable which contains a library whose
name contains 'vgpreload'. If it sees this, LXC driver totally disables
itself. This lets me reliably valgrind the libvirtd daemon again.
Second, I also move the lxcProbe() call in the lxcOpen() method down a
little, so we only probe if we are actually about to try opening the
connection. This avoids some unneccessary container creation checks
for most virConnectOpen scenarios.
Daniel
diff -r 6c8e581563aa src/lxc_driver.c
--- a/src/lxc_driver.c Fri Jan 30 11:00:43 2009 +
+++ b/src/lxc_driver.c Fri Jan 30 11:01:10 2009 +
@@ -80,14 +80,14 @@ static virDrvOpenStatus lxcOpen(virConne
virConnectAuthPtr auth ATTRIBUTE_UNUSED,
int flags ATTRIBUTE_UNUSED)
{
-if (!lxcProbe())
-goto declineConnection;
-
if (lxc_driver == NULL)
goto declineConnection;
/* Verify uri was specified */
if (conn-uri == NULL) {
+if (!lxcProbe())
+goto declineConnection;
+
conn-uri = xmlParseURI(lxc:///);
if (!conn-uri) {
virReportOOMError(conn);
@@ -96,8 +96,11 @@ static virDrvOpenStatus lxcOpen(virConne
} else if (conn-uri-scheme == NULL ||
STRNEQ(conn-uri-scheme, lxc)) {
goto declineConnection;
+} else if (!lxcProbe()) {
+goto declineConnection;
}
+
conn-privateData = lxc_driver;
return VIR_DRV_OPEN_SUCCESS;
@@ -1119,6 +1122,13 @@ static int lxcStartup(void)
{
uid_t uid = getuid();
unsigned int i;
+char *ld;
+
+/* Valgrind gets very annoyed when we clone containers, so
+ * disable LXC when under valgrind */
+ld = getenv(LD_PRELOAD);
+if (ld strstr(ld, vgpreload))
+return -1;
/* Check that the user is root */
if (0 != uid) {
--
|: 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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: Fix leak in storage driver
A recent change to keep the storage pools active upon shutdown, exposed a minor flaw in the code which free's a virStoragePoolObj instance. It never free's the associated volumes, since it presumed you'd never free a pool, which was still active. A bogus assumption, causing us to leak memory upon daemon shutdown, thus annoying valgrind. Daniel diff -r 3e95abd6df89 src/storage_conf.c --- a/src/storage_conf.cFri Jan 30 11:01:10 2009 + +++ b/src/storage_conf.cFri Jan 30 11:01:29 2009 + @@ -296,6 +296,8 @@ virStoragePoolObjFree(virStoragePoolObjP if (!obj) return; +virStoragePoolObjClearVols(obj); + virStoragePoolDefFree(obj-def); virStoragePoolDefFree(obj-newDef); -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: Fix leak in libvirtd daemon when reporting errors
When reporting libvirt errors back to the client, we forgot to free the memory associated with the remote_error object - principally the char * message strings. So every error reported would leak some memory. This patch free's the memory Daniel diff -r 1bd5f9fd5393 qemud/remote.c --- a/qemud/remote.cFri Jan 30 11:01:29 2009 + +++ b/qemud/remote.cFri Jan 30 11:01:52 2009 + @@ -363,6 +363,7 @@ rpc_error: remoteDispatchGenericError(rerr); if (!xdr_remote_error (xdr, rerr)) goto fatal_error; +xdr_free((xdrproc_t)xdr_remote_error, (char *)rerr); } /* Write the length word. */ -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt for rhel-5?
On Wed, Jan 28, 2009 at 12:32 PM, Daniel P. Berrange berra...@redhat.com wrote: NB, if you're re-packaging libvirt for EPEL-5, be warned that none of http://fedoraproject.org/wiki/EPEL/FAQ#Does_EPEL_replace_packages_provided_within_Red_Hat_Enterprise_Linux_or_layered_products.3F libvirt cannot be in EPEL-5 since it's in baseOS -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt for rhel-5?
Alan Pevec wrote: On Wed, Jan 28, 2009 at 12:32 PM, Daniel P. Berrange berra...@redhat.com wrote: NB, if you're re-packaging libvirt for EPEL-5, be warned that none of http://fedoraproject.org/wiki/EPEL/FAQ#Does_EPEL_replace_packages_provided_within_Red_Hat_Enterprise_Linux_or_layered_products.3F libvirt cannot be in EPEL-5 since it's in baseOS no, i'd like to add to my onw repo and may be later to centos's extras repo. the current rhel-5 kvm support is simple unusable:-( you can only use xen in that case. and unfortunately as rhel don't want updat their kernel in rhel-5 kvm support never will really good. -- Levente Si vis pacem para bellum! -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
On Fri, Jan 30, 2009 at 12:07:31AM +0100, Jim Meyering wrote:
Here's your rebased and adjusted patch:
From ce4f15853e119d6d976a5d29917f62f577e8ec9e Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 29 Jan 2009 22:50:36 +0100
Subject: [PATCH] allow disk cache mode to be specified in a domain's xml
definition.
Daniel P. Berrange wrote:
If you loook at src/qemu_conf.c, you'll find a nice method called
qemudExtractVersionInfo, which runs 'qemu -help' and checks for
certain interesting command line arguments :-)
That problem does seem to be crying for some type
of structured interface to avoid subtle breakage
should someone modify the output of --help.
I'm sure I'm preaching to the choir here.
So it now adapts for the cases of old syntax and
writethrough as well as new syntax and on
since qemu will otherwise balk at those cache
flag / version combinations.
One note about the enums - rather than adding old style CACHEON
CACHE_OFF options to the main enum in domain_conf, just create
a second enum in the qemu_conf.c file for recording the mapping
of virDomainDiskCache values to old style QEMU arguments values..
As we are adapting in both directions I left the
single enum representing the entire option list
to simplify things. Updated patch is attached.
---
src/domain_conf.c | 34 ++
src/domain_conf.h | 16
src/qemu_conf.c | 41 ++---
src/qemu_conf.h |3 ++-
4 files changed, 82 insertions(+), 12 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index f696b6a..efd6981 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -1,7 +1,7 @@
/*
* domain_conf.c: domain XML processing
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -551,6 +551,17 @@ int virDomainDiskCompare(virDomainDiskDefPtr a,
#ifndef PROXY
+
+/* map from xml cache tag to internal cache mode
+ */
+VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
+, /* reserved -- mode not specified */
+off,
+on,
+none,
+writeback,
+writethrough);
This is still wrong - the XML should only accept 'none',
'writeback', and 'writethrough', or 'default'. It shouldn't
allow QEMU's 'on' or 'off' values - that's an internal impl
detail
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 09afd1f..c72c0dc 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -81,6 +81,21 @@ enum virDomainDiskBus {
VIR_DOMAIN_DISK_BUS_LAST
};
+/* summary of all possible host open file cache modes
+ */
+typedef enum {
+VIR_DOMAIN_DISK_CACHE_UNSPECIFIED, /* reserved */
+VIR_DOMAIN_DISK_CACHE_OFF,
+VIR_DOMAIN_DISK_CACHE_ON,
+VIR_DOMAIN_DISK_CACHE_DISABLE,
+VIR_DOMAIN_DISK_CACHE_WRITEBACK,
+VIR_DOMAIN_DISK_CACHE_WRITETHRU,
+
+VIR_DOMAIN_DISK_CACHE_LAST
+} virDomainDiskCache;
Likewise ON/OFF should not be used here.
+VIR_ENUM_DECL(qemu_cache_map)
+
+/* map from internal cache mode to qemu cache arg text
+ *
+ * Note: currently this replicates virDomainDiskCache, but will need to
+ * error flag potential new entries in virDomainDiskCache which are
+ * not supported by qemu (raising exceptions as appropriate).
+ */
+VIR_ENUM_IMPL(qemu_cache_map, VIR_DOMAIN_DISK_CACHE_LAST,
+, /* reserved -- mode not specified */
+off, /* deprecated; use none */
+on, /* obsolete; use writethrough */
+none,
+writeback,
+writethrough)
This needs to have two separate enums, one for old style, one for
new style option naming
@@ -1012,6 +1030,24 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (disk-driverType)
virBufferVSprintf(opt, ,fmt=%s, disk-driverType);
+unsigned int qf;
+int cachemode = disk-cachemode;
+if (cachemode) {
+if (qemudExtractVersionInfo(vm-def-emulator, NULL, qf)
0)
+;/* error reported */
This will SEGV, because vm-def-emulator is not required to be present
here. The command line flags are already extract available further
up in this method, so its redundant to extract them again.
+else if (!(qf QEMUD_CMD_FLAG_DRIVE_CACHEMODE)
+ cachemode == VIR_DOMAIN_DISK_CACHE_WRITETHRU)
+cachemode = VIR_DOMAIN_DISK_CACHE_ON;
+else if (qf QEMUD_CMD_FLAG_DRIVE_CACHEMODE
+ cachemode == VIR_DOMAIN_DISK_CACHE_ON)
+cachemode = VIR_DOMAIN_DISK_CACHE_WRITETHRU;
+}
+if (cachemode || (disk-shared !disk-readonly))
+virBufferVSprintf(opt, ,cache=%s,
+
Re: [libvirt] libvirt for rhel-5?
On Fri, Jan 30, 2009 at 01:00:25PM +0100, Farkas Levente wrote: Alan Pevec wrote: On Wed, Jan 28, 2009 at 12:32 PM, Daniel P. Berrange berra...@redhat.com wrote: NB, if you're re-packaging libvirt for EPEL-5, be warned that none of http://fedoraproject.org/wiki/EPEL/FAQ#Does_EPEL_replace_packages_provided_within_Red_Hat_Enterprise_Linux_or_layered_products.3F libvirt cannot be in EPEL-5 since it's in baseOS no, i'd like to add to my onw repo and may be later to centos's extras repo. the current rhel-5 kvm support is simple unusable:-( you can only use xen in that case. and unfortunately as rhel don't want updat their kernel in rhel-5 kvm support never will really good. To clarify there is *NO* RHEL-5 KVM support at all. It is not part of RHEL-5. So it say it is unusable is to criticise something which does not exist. Any KVM support found in RHEL-5 is something added by a 3rd party. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] (resend) Problems with virt-manager checking access on virtual images.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Met with Cole this morning and we talked about how SELinux can cause
people headaches when installing virtual images from random locations.
User downloads a iso image to his home directory and then uses
virt-manager to install it. Problem is when the user has the whole
thing configured, virt-manager tells libvirt to install. It executes
qemu and SELinux prevents qemu from reading the iso image because it is
labeled user_home_t and qemu is not allowed to read the contents of the
home directory. qemu blows up with permission denied and the user is at
a loss to what just happened.
As we talked we realised this is not just an SELinux problem, but would
also happen if a use had an nfs homedir or potentially a samba home
directory where root was not allowed access. Also pam_namespace would
cause problems, in the /tmp or /home/dwalsh would not be the same for
root as they are for the user.
One solution to the SELinux problem is to have a label that virt-manager
could apply to the iso image (virt_content_ro_t). This would allow qemu
to access the file as long as it had search access to the path to the
image. solving most of these problems. But the user could still have
an access problem that would be tough to diagnose. We came up with the
idea of a running a simple helper application to check read access to
the image file. During the install, virt-manager could tell libvirt to
verify access by executing qemuaccess /home/dwalsh/windows.iso. If
this executable was labeled qemu_exec_t like the other qemu images the
same SELinux transitions would happen and we could instantly figure out
if SELinux was going to cause problems. As a side benefit we could also
check if NFS or samba would cause a problems. If qemuaccess failed,
virt-manager could put up a diagnostic message suggesting SELinux, NFS,
or Samba might be a problem, and the user could move the iso image to
some directory like /var/lib/libvirt/isos/, where libirt would have access.
I have attached a version that could solve the problem.
Comments?
Dan
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iEYEARECAAYFAkmC9NAACgkQrlYvE4MpobO8egCgpOlWtlSSrC+TPK41fWC9YPWg
xwoAn2zYpk5ODoGhl5PXnwkltBKVjO1m
=PYqR
-END PGP SIGNATURE-
#include stdio.h
#include stdlib.h
#include string.h
#include errno.h
main(int argc, char **argv) {
int rc = 0;
int i=0;
if (argc 2) {
fprintf(stderr, %s: image file(s) required\n, argv[0]);
exit(-1);
}
for(i=1; i argc; i++) {
FILE *fp = fopen(argv[i], r);
if (!fp) {
fprintf(stderr, %s: Can not read %s: %s\n, argv[0], argv[i], strerror(errno));
rc = -1;
} else {
fclose(fp);
}
}
return rc;
}
qemuaccess.c.sig
Description: Binary data
qemuaccess.c.sig
Description: PGP signature
qemuaccess.c.sig.sig
Description: PGP signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] (resend) Problems with virt-manager checking access on virtual images.
On Fri, Jan 30, 2009 at 07:38:40AM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Met with Cole this morning and we talked about how SELinux can cause people headaches when installing virtual images from random locations. User downloads a iso image to his home directory and then uses virt-manager to install it. Problem is when the user has the whole thing configured, virt-manager tells libvirt to install. It executes qemu and SELinux prevents qemu from reading the iso image because it is labeled user_home_t and qemu is not allowed to read the contents of the home directory. qemu blows up with permission denied and the user is at a loss to what just happened. As we talked we realised this is not just an SELinux problem, but would also happen if a use had an nfs homedir or potentially a samba home directory where root was not allowed access. Also pam_namespace would cause problems, in the /tmp or /home/dwalsh would not be the same for root as they are for the user. One solution to the SELinux problem is to have a label that virt-manager could apply to the iso image (virt_content_ro_t). This would allow qemu to access the file as long as it had search access to the path to the image. solving most of these problems. But the user could still have an access problem that would be tough to diagnose. We came up with the idea of a running a simple helper application to check read access to the image file. During the install, virt-manager could tell libvirt to verify access by executing qemuaccess /home/dwalsh/windows.iso. If this executable was labeled qemu_exec_t like the other qemu images the same SELinux transitions would happen and we could instantly figure out if SELinux was going to cause problems. As a side benefit we could also check if NFS or samba would cause a problems. If qemuaccess failed, virt-manager could put up a diagnostic message suggesting SELinux, NFS, or Samba might be a problem, and the user could move the iso image to some directory like /var/lib/libvirt/isos/, where libirt would have access. I don't particularly like the idea of running another program to check this because SELinux context isn't the only thing which will potentially prevent QEMU accessing the disk image. CGroups device policy make prevent it. A setuid() call in QEMU to drop privileges, may prevent it. It may be asked to open it read-write, and only be able to open it read-only . It may want to take a lock on the image, but it already be locked, and so on. QEMU does already report pretty much the same error message as the qemuaccess program does when it is unable to access a disk image # virsh start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso IMHO, this all stems from a mistake I made when designing the original virt-manager UI. Namely, I should never have used a generic file dialog which allows selection of disk images anywhere on the host. It was wrong for general disk images, in just the same way that its wrong for ISO images. And absolutely useless for remote provisioning. With the storage pool APIs we have the opportunity to fix this in a way helps not only the SELinux case, but also the app in general. Instead of showing a generic file dialog, we should just show a dialog displaying the configured storage pools, and allow the user to pick an ISO out of one of the the pools. virt-manager should offer to remember which storage pool contains installable ISO images, and which should be used by default for disk images. We already have /var/lib/libvirt/images by default for disk images, and should add /var/lib/libvirt/isos too. If a user downloads an ISO to their home directory, we could easily have a option in the UI for 'Import an ISO file to the pool' which would move it into the correct location. NB, here I am talking about use of the system-wide QEMU driver instance of 'qemu:///system', which runs privileged on the host. This is really intended at server deployments of virt, but we have been happening to use it for general ad-hoc desktop usage too in Fedora. There is also a per-user 'qemu://session' UI where both libvirtd and the QEMU guests run unprivileged as that user's UID. In that scenario the storage pools should use something like $HOME/VirtualMachines/images and $HOME/VirtualMachines/ISOs'as the default pools for disk ISOs respectively. I would like to see Fedora use qemu:///session by default for generic desktop virt usage, in which case everything runs as that UID, and keeps everything in $HOME. Regards, Daniel -- |: 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 :| --
[libvirt] PATCH: Fix passing of ifname to QEMU
When using the type='ethernet' network device configuration for a guest
we pass a script, and optional interface name to QEMU. If ifname is
omitted, then QEMU allocates one itself. The problem was we were passing
an ifname of '(null)' by mistake. This patch corrects that problem and
adds a test for it.
Daniel
diff -r b6a065030fa6 src/qemu_conf.c
--- a/src/qemu_conf.c Fri Jan 30 12:28:00 2009 +
+++ b/src/qemu_conf.c Fri Jan 30 13:07:11 2009 +
@@ -1147,11 +1147,18 @@ int qemudBuildCommandLine(virConnectPtr
case VIR_DOMAIN_NET_TYPE_ETHERNET:
{
char arg[PATH_MAX];
-if (snprintf(arg, PATH_MAX-1,
tap,ifname=%s,script=%s,vlan=%d,
- net-ifname,
- net-data.ethernet.script,
- vlan) = (PATH_MAX-1))
-goto error;
+if (net-ifname) {
+if (snprintf(arg, PATH_MAX-1,
tap,ifname=%s,script=%s,vlan=%d,
+ net-ifname,
+ net-data.ethernet.script,
+ vlan) = (PATH_MAX-1))
+goto error;
+} else {
+if (snprintf(arg, PATH_MAX-1, tap,script=%s,vlan=%d,
+ net-data.ethernet.script,
+ vlan) = (PATH_MAX-1))
+goto error;
+}
ADD_ARG_LIT(arg);
}
diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args Fri Jan 30
13:07:11 2009 +
@@ -0,0 +1,1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M
pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid
-no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net
nic,macaddr=00:11:22:33:44:55,vlan=0 -net
tap,ifname=nic02,script=/etc/qemu-ifup,vlan=0 -serial none -parallel none -usb
diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xmlFri Jan 30
13:07:11 2009 +
@@ -0,0 +1,27 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory219200/memory
+ currentMemory219200/currentMemory
+ vcpu1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+ source dev='/dev/HostVG/QEMUGuest1'/
+ target dev='hda' bus='ide'/
+/disk
+interface type='ethernet'
+ mac address='00:11:22:33:44:55'/
+ script path='/etc/qemu-ifup'/
+ target dev='nic02'/
+/interface
+ /devices
+/domain
diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth.args
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args Fri Jan 30 13:07:11
2009 +
@@ -0,0 +1,1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M
pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid
-no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net
nic,macaddr=00:11:22:33:44:55,vlan=0 -net tap,script=/etc/qemu-ifup,vlan=0
-serial none -parallel none -usb
diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml Fri Jan 30 13:07:11
2009 +
@@ -0,0 +1,26 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory219200/memory
+ currentMemory219200/currentMemory
+ vcpu1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+ source dev='/dev/HostVG/QEMUGuest1'/
+ target dev='hda' bus='ide'/
+/disk
+interface type='ethernet'
+ mac address='00:11:22:33:44:55'/
+ script path='/etc/qemu-ifup'/
+/interface
+ /devices
+/domain
diff -r b6a065030fa6 tests/qemuxml2argvtest.c
--- a/tests/qemuxml2argvtest.c Fri Jan 30 12:28:00 2009 +
+++ b/tests/qemuxml2argvtest.c Fri Jan 30 13:07:11 2009 +
@@ -224,6 +224,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_UUID | QEMUD_CMD_FLAG_DOMID);
DO_TEST(net-user, 0);
DO_TEST(net-virtio, 0);
+DO_TEST(net-eth, 0);
+DO_TEST(net-eth-ifname, 0);
DO_TEST(serial-vc, 0);
[libvirt] Re: [PATCH 0/3] A small example program
Hi David, I've just applied your 3 patches and built the example. That went fine. Then I tried to run it like this: $ LIBVIRT_DEBUG=1 libvirtd 2 log pid=$! $ ./hellolibvirt Attempting to connect to hypervisor Connected to hypervisor at qemu:///session ? - Failed to get hypervisor version Disconnected from hypervisor [Exit 1] There was plenty in the log: DEBUG: libvirt.c: virInitialize (register drivers) DEBUG: libvirt.c: virRegisterDriver (registering Test as driver 0) DEBUG: libvirt.c: virRegisterNetworkDriver (registering Test as network driver 0) DEBUG: libvirt.c: virRegisterStorageDriver (registering Test as storage driver 0) DEBUG: libvirt.c: virRegisterDriver (registering Xen as driver 1) DEBUG: libvirt.c: virRegisterDriver (registering OPENVZ as driver 2) DEBUG: libvirt.c: virRegisterDriver (registering remote as driver 3) DEBUG: libvirt.c: virRegisterNetworkDriver (registering remote as network driver 1) DEBUG: libvirt.c: virRegisterStorageDriver (registering remote as storage driver 1) DEBUG: libvirt.c: virRegisterDeviceMonitor (registering remote as device driver 0) DEBUG: libvirt.c: virRegisterNetworkDriver (registering Network as network driver 2) DEBUG: libvirt.c: virRegisterStorageDriver (registering storage as storage driver 2) DEBUG: libvirt.c: virRegisterDeviceMonitor (registering halDeviceMonitor as device driver 1) DEBUG: libvirt.c: virRegisterDriver (registering QEMU as driver 4) DEBUG: libvirt.c: virRegisterDriver (registering LXC as driver 5) DEBUG: libvirt.c: virRegisterDriver (registering UML as driver 6) DEBUG: libvirt.c: virConnectOpen (name=) DEBUG: libvirt.c: do_open (no name, allowing driver auto-select) DEBUG: libvirt.c: do_open (trying driver 0 (Test) ...) DEBUG: libvirt.c: do_open (driver 0 Test returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 1 (Xen) ...) DEBUG: libvirt.c: do_open (driver 1 Xen returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 2 (OPENVZ) ...) DEBUG: libvirt.c: do_open (driver 2 OPENVZ returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 3 (remote) ...) DEBUG: libvirt.c: do_open (driver 3 remote returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 4 (QEMU) ...) DEBUG: libvirt.c: do_open (driver 4 QEMU returned SUCCESS) DEBUG: libvirt.c: do_open (network driver 0 Test returned DECLINED) DEBUG: libvirt.c: do_open (network driver 1 remote returned DECLINED) DEBUG: libvirt.c: do_open (network driver 2 Network returned SUCCESS) DEBUG: libvirt.c: do_open (storage driver 0 Test returned DECLINED) DEBUG: libvirt.c: do_open (storage driver 1 remote returned DECLINED) DEBUG: libvirt.c: do_open (storage driver 2 storage returned SUCCESS) DEBUG: libvirt.c: do_open (node driver 0 remote returned DECLINED) DEBUG: libvirt.c: do_open (node driver 1 halDeviceMonitor returned SUCCESS) DEBUG: libvirt.c: virConnectGetURI (conn=0xcbab10) DEBUG: libvirt.c: virConnectGetType (conn=0xcbab10) DEBUG: libvirt.c: virConnectGetVersion (conn=0xcbab10, hvVer=0x7fffbc541108) DEBUG: libvirt.c: virConnectClose (conn=0xcbab10) DEBUG: datatypes.c: virUnrefConnect (unref connection 0xcbab10 1) DEBUG: datatypes.c: virReleaseConnect (release connection 0xcbab10) Shutting down on signal 15 Then I repeated with the just-built libvirtd, instead of the installed one, but then I get a new error: $ kill $pid $ LIBVIRT_DEBUG=1 ../../qemud/libvirtd 2 log $ ./hellolibvirt Attempting to connect to hypervisor Connected to hypervisor at qemu:///session ? - libvir: error : Unknown failure Failed to get hypervisor version Disconnected from hypervisor [Exit 1] This is on F10 x86_64. Does the above work for you? Also, please change examples/hellolibvirt/Makefile.am to use $(...) rather than the obsolete @...@ notation. I.e., just apply the following to the rest when you squash those commits into one. Yes, I know there are quite a few existing uses of @...@, and I even wrote a patch to remove them, but made the mistake of also fixing some minor quoting bugs along the way, and haven't gotten around to separating it into two change sets. Finally, please add examples/hellolibvirt/hellolibvirt to the top-level .cvsignore and rerun sync-vcs-ignore-files to regenerate .gitignore and .hgignore to match. From 27f0007067280e4fa93b8c1d3c1c203314bde285 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 30 Jan 2009 10:02:51 +0100 Subject: [PATCH] use $(var), rather than obsolete @var@ notation --- examples/hellolibvirt/Makefile.am |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/hellolibvirt/Makefile.am b/examples/hellolibvirt/Makefile.am index 6ef2cc8..489bf8b 100644 --- a/examples/hellolibvirt/Makefile.am +++ b/examples/hellolibvirt/Makefile.am @@ -1,5 +1,5 @@ -INCLUDES = -...@top_srcdir@/include +INCLUDES = -I$(top_srcdir)/include noinst_PROGRAMS = hellolibvirt hellolibvirt_CFLAGS = $(WARN_CFLAGS) hellolibvirt_SOURCES =
Re: [libvirt] Re: [PATCH 0/3] A small example program
Jim Meyering j...@meyering.net wrote:
I've just applied your 3 patches and built the example.
That went fine. Then I tried to run it like this:
...
$ kill $pid
$ LIBVIRT_DEBUG=1 ../../qemud/libvirtd 2 log
$ ./hellolibvirt
Attempting to connect to hypervisor
Connected to hypervisor at qemu:///session
? - libvir: error : Unknown failure
Failed to get hypervisor version
Disconnected from hypervisor
[Exit 1]
This is on F10 x86_64.
FYI, I poked around in the server to see what was going wrong.
qemudGetVersion calls qemudExtractVersion, which calls
virCapabilitiesDefaultGuestEmulator, which compares the
single guest cap and gets an arch mismatch:
Breakpoint 1, virCapabilitiesDefaultGuestEmulator (caps=0x71fee0,
ostype=0x4bbc40 hvm, arch=0x4bb980 i686, domain=0x4bbc44 qemu)
at capabilities.c:497
...
(gdb) p *caps
$12 = {
host = {
arch = 0x71be60 x86_64,
nfeatures = 0,
features = 0x0,
offlineMigrate = 0,
liveMigrate = 0,
nmigrateTrans = 0,
migrateTrans = 0x0,
nnumaCell = 1,
numaCell = 0x717430
},
So this test fails: x86_64 != i686
STREQ(caps-guests[i]-arch.name, arch)
so qemudGetVersion does, too.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH 0/3] A small example program
On Fri, Jan 30, 2009 at 02:32:00PM +0100, Jim Meyering wrote: Jim Meyering j...@meyering.net wrote: I've just applied your 3 patches and built the example. That went fine. Then I tried to run it like this: ... $ kill $pid $ LIBVIRT_DEBUG=1 ../../qemud/libvirtd 2 log $ ./hellolibvirt Attempting to connect to hypervisor Connected to hypervisor at qemu:///session ? - libvir: error : Unknown failure Failed to get hypervisor version Disconnected from hypervisor [Exit 1] This is on F10 x86_64. FYI, I poked around in the server to see what was going wrong. qemudGetVersion calls qemudExtractVersion, which calls virCapabilitiesDefaultGuestEmulator, which compares the single guest cap and gets an arch mismatch: Hmm, it should use the native arch - virExtractVersionInfo needs fixing to call uname, and extract the native arch. Hardcoding i386 was sufficient, when we mandated that 'qemu' was always present, but now we allow either qemu or KVM, so we need to always use native. NB, i3/4/586 should be mapped to i686. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH 0/3] A small example program
On Fri, Jan 30, 2009 at 02:10:58PM +0100, Jim Meyering wrote: Hi David, I've just applied your 3 patches and built the example. That went fine. Then I tried to run it like this: $ LIBVIRT_DEBUG=1 libvirtd 2 log pid=$! $ ./hellolibvirt Attempting to connect to hypervisor Connected to hypervisor at qemu:///session ? - Failed to get hypervisor version This shows a place where the example program could use the libvirt error information - it should have received error message to the effect of 'Cannot find QEMU binary /usr/bin/qemu' when qemudExtractVersion failed to find the binary it wanted Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] (resend) Problems with virt-manager checking access on virtual images.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Fri, Jan 30, 2009 at 07:38:40AM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Met with Cole this morning and we talked about how SELinux can cause people headaches when installing virtual images from random locations. User downloads a iso image to his home directory and then uses virt-manager to install it. Problem is when the user has the whole thing configured, virt-manager tells libvirt to install. It executes qemu and SELinux prevents qemu from reading the iso image because it is labeled user_home_t and qemu is not allowed to read the contents of the home directory. qemu blows up with permission denied and the user is at a loss to what just happened. As we talked we realised this is not just an SELinux problem, but would also happen if a use had an nfs homedir or potentially a samba home directory where root was not allowed access. Also pam_namespace would cause problems, in the /tmp or /home/dwalsh would not be the same for root as they are for the user. One solution to the SELinux problem is to have a label that virt-manager could apply to the iso image (virt_content_ro_t). This would allow qemu to access the file as long as it had search access to the path to the image. solving most of these problems. But the user could still have an access problem that would be tough to diagnose. We came up with the idea of a running a simple helper application to check read access to the image file. During the install, virt-manager could tell libvirt to verify access by executing qemuaccess /home/dwalsh/windows.iso. If this executable was labeled qemu_exec_t like the other qemu images the same SELinux transitions would happen and we could instantly figure out if SELinux was going to cause problems. As a side benefit we could also check if NFS or samba would cause a problems. If qemuaccess failed, virt-manager could put up a diagnostic message suggesting SELinux, NFS, or Samba might be a problem, and the user could move the iso image to some directory like /var/lib/libvirt/isos/, where libirt would have access. I don't particularly like the idea of running another program to check this because SELinux context isn't the only thing which will potentially prevent QEMU accessing the disk image. CGroups device policy make prevent it. A setuid() call in QEMU to drop privileges, may prevent it. It may be asked to open it read-write, and only be able to open it read-only . It may want to take a lock on the image, but it already be locked, and so on. QEMU does already report pretty much the same error message as the qemuaccess program does when it is unable to access a disk image # virsh start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso IMHO, this all stems from a mistake I made when designing the original virt-manager UI. Namely, I should never have used a generic file dialog which allows selection of disk images anywhere on the host. It was wrong for general disk images, in just the same way that its wrong for ISO images. And absolutely useless for remote provisioning. With the storage pool APIs we have the opportunity to fix this in a way helps not only the SELinux case, but also the app in general. Instead of showing a generic file dialog, we should just show a dialog displaying the configured storage pools, and allow the user to pick an ISO out of one of the the pools. virt-manager should offer to remember which storage pool contains installable ISO images, and which should be used by default for disk images. We already have /var/lib/libvirt/images by default for disk images, and should add /var/lib/libvirt/isos too. If a user downloads an ISO to their home directory, we could easily have a option in the UI for 'Import an ISO file to the pool' which would move it into the correct location. NB, here I am talking about use of the system-wide QEMU driver instance of 'qemu:///system', which runs privileged on the host. This is really intended at server deployments of virt, but we have been happening to use it for general ad-hoc desktop usage too in Fedora. There is also a per-user 'qemu://session' UI where both libvirtd and the QEMU guests run unprivileged as that user's UID. In that scenario the storage pools should use something like $HOME/VirtualMachines/images and $HOME/VirtualMachines/ISOs'as the default pools for disk ISOs respectively. I would like to see Fedora use qemu:///session by default for generic desktop virt usage, in which case everything runs as that UID, and keeps everything in $HOME. Regards, Daniel We discussed this also. The only problem I see here is the additional disk usage, since you would permanently keeping the iso, even though you might only be using it once. What about removable media,
Re: [libvirt] PATCH: Fix infinite loop when QEMU quits at startup
On Fri, Jan 30, 2009 at 11:37:35AM +, Daniel P. Berrange wrote: The recent refactoring of the QEMU startup process now reads the monitor TTY from the logfile. Unfortunately in this refactoring we lost the check for the 'ret == 0' scenario in the read() return value. So if QEMU quits at startup, eg due to missing disk image, we loop forever on read() == 0 because we hit end-of-file and QEMU has quit. This patch adds back handling for this scenario, and takes care to propagate the contents of the log to the user as an error message # start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso error: Failed to start domain demo In addition, there were a couple of other bugs - a memory leak where we set the 'monitorpath' variable, even though we'd just set it moments before. - a missing check for whether the driver VNC password was present when initializing passwords at VM startupo - missing initialization of the monitor_watch field, and missing checking for whether the watch was set before removing it. - a gratuitous LOG_INFO when shutting down any VM, which could just be LOG_DEBUG. Patch looks reasonnable to me, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
Daniel P. Berrange berra...@redhat.com wrote: ... Further up in this code its already adding cache=off for shared disks. Probably introduced by me in the merge. There's also quite a few code style issues, not following conventions of the surrounding code. Here's a patch which addresses all that, and more importantly adds test cases covering all the cache variants QEMU has. b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml | 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml | 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args|1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml | 30 +++ I looked through the incremental changes, and they look ok, but see that they do introduce new test failures: 4) qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml FAILED Relax-NG validity error : Extra element devices in interleave /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml:15: element devices: Relax-NG validity error : Element domain failed to validate content /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml fails to validate ... 6) qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml FAILED Relax-NG validity error : Extra element devices in interleave /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml:15: element devices: Relax-NG validity error : Element domain failed to validate content /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml fails to validate ... 14) qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml FAILED Relax-NG validity error : Extra element devices in interleave /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml:15: element devices: Relax-NG validity error : Element domain failed to validate content /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml fails to validate ... ^R 17) qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml FAILED Relax-NG validity error : Extra element devices in interleave /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml:15: element devices: Relax-NG validity error : Element domain failed to validate content /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml fails to validate ... 22) qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml FAILED Relax-NG validity error : Extra element devices in interleave /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml:15: element devices: Relax-NG validity error : Element domain failed to validate content /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml fails to validate ... 42) qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.xml FAILED Relax-NG validity error : Extra element devices in interleave /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.xml:15: element devices: Relax-NG validity error : Element domain failed to validate content /c/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.xml fails to validate -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] (resend) Problems with virt-manager checking access on virtual images.
On Fri, Jan 30, 2009 at 09:06:38AM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: I don't particularly like the idea of running another program to check this because SELinux context isn't the only thing which will potentially prevent QEMU accessing the disk image. CGroups device policy make prevent it. A setuid() call in QEMU to drop privileges, may prevent it. It may be asked to open it read-write, and only be able to open it read-only . It may want to take a lock on the image, but it already be locked, and so on. QEMU does already report pretty much the same error message as the qemuaccess program does when it is unable to access a disk image # virsh start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso IMHO, this all stems from a mistake I made when designing the original virt-manager UI. Namely, I should never have used a generic file dialog which allows selection of disk images anywhere on the host. It was wrong for general disk images, in just the same way that its wrong for ISO images. And absolutely useless for remote provisioning. With the storage pool APIs we have the opportunity to fix this in a way helps not only the SELinux case, but also the app in general. Instead of showing a generic file dialog, we should just show a dialog displaying the configured storage pools, and allow the user to pick an ISO out of one of the the pools. virt-manager should offer to remember which storage pool contains installable ISO images, and which should be used by default for disk images. We already have /var/lib/libvirt/images by default for disk images, and should add /var/lib/libvirt/isos too. If a user downloads an ISO to their home directory, we could easily have a option in the UI for 'Import an ISO file to the pool' which would move it into the correct location. NB, here I am talking about use of the system-wide QEMU driver instance of 'qemu:///system', which runs privileged on the host. This is really intended at server deployments of virt, but we have been happening to use it for general ad-hoc desktop usage too in Fedora. There is also a per-user 'qemu://session' UI where both libvirtd and the QEMU guests run unprivileged as that user's UID. In that scenario the storage pools should use something like $HOME/VirtualMachines/images and $HOME/VirtualMachines/ISOs'as the default pools for disk ISOs respectively. I would like to see Fedora use qemu:///session by default for generic desktop virt usage, in which case everything runs as that UID, and keeps everything in $HOME. We discussed this also. The only problem I see here is the additional disk usage, since you would permanently keeping the iso, even though you might only be using it once. What about removable media, cdrom and usb key isos. Do you want to copy them to the /var/lib/libvirt/isos directory also? If think if someone downloads a large ISO its fairly likely they'll want to keep it around for some amount of time, rather than risk having to download it again. That said, our storage pools APIs can trivally be used to delete it if it is no longer required. For removable media, we're just directly attaching the block device, so we can't avoid the need to re-label from fixed_dev_t (or equiva) to virt_image_t - though having a virt_image_ro_t for readonly access would be preferrable for cases where we're explicitly attaching it to the guest in read-only mode. USB keys I'm not so sure about - in the common case they'll be FAT filesystem which doesn't support labelling at all. So we'd either have to ensure its mounted with a suitable context= mount option, or get the policy to to allow read-only access to the default mount context for FAT filesystems. The /var/lib/libvirt/isos directly is just a plain directory based storage pool. We also have concept of filesystem based storage pools (eg a local device, mounted in some directory) which can we directly manage. Likewise NFS mounts we can handle directly, mounting a remote directoy from a server in a specific place, so we can ensure a context= mount option is used. So I think a combination of factors - Default directories for local host stored file based images ISO which get correct context automatically - libvirt has to set contenxt on any block device nodes which need to be assigned - Setting mount context for removable disks / changing policy to allow read-only access to mounted removable disks. Are the isos in this directory going to be Read Only or can some qemus read/write them? Any files used to back the QEMU CDROM device should be restricted to be read-only, since we need ability to safely assign the same ISO to multiple guests concurrently - for concurrent installs. So a new disk type virt_image_ro_t might be
Re: [libvirt] PATCH: Avoid crashing valgrind in LXC driver
On Fri, Jan 30, 2009 at 11:41:16AM +, Daniel P. Berrange wrote: The LXC driver makes use of new clone flags for creating containers. It creates a dummy container which immediately exits in order to test for availabilty of this feature in the kernel. Unfortunately valgrind has no knowledge of these new clone flags, gets very very unhappy and then reports bogus memory leaks, bogus illegal instructions, and then often SEGV's itself. Not cool. While obviously valgrind needs fixing, I looked at its code, and it doesn't seem easy, so this patch adds a quick check for a LD_PRELOAD environemnt variable which contains a library whose name contains 'vgpreload'. If it sees this, LXC driver totally disables itself. This lets me reliably valgrind the libvirtd daemon again. Okay, this is a workaround hack, I'm fine applying it but we should log this somewhere (TODO ?) to think about cleaning this up when valgrind get fixed. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Fix leak in libvirtd daemon when reporting errors
On Fri, Jan 30, 2009 at 11:44:35AM +, Daniel P. Berrange wrote: When reporting libvirt errors back to the client, we forgot to free the memory associated with the remote_error object - principally the char * message strings. So every error reported would leak some memory. This patch free's the memory Sounds a fairly important fix, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Fix passing of ifname to QEMU
On Fri, Jan 30, 2009 at 01:08:54PM +, Daniel P. Berrange wrote: When using the type='ethernet' network device configuration for a guest we pass a script, and optional interface name to QEMU. If ifname is omitted, then QEMU allocates one itself. The problem was we were passing an ifname of '(null)' by mistake. This patch corrects that problem and adds a test for it. Okay, we would have caught it before if the libc segfaulted in such a situation. +1 to the patch . I wonder if there is a way to avoid glibc current behaviour when doing regression tests, waiting for solaris testing is not a good way to catch those :-\ Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
On Fri, Jan 30, 2009 at 03:27:55PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... Further up in this code its already adding cache=off for shared disks. Probably introduced by me in the merge. There's also quite a few code style issues, not following conventions of the surrounding code. Here's a patch which addresses all that, and more importantly adds test cases covering all the cache variants QEMU has. b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml | 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml | 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.args |1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml| 29 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args|1 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml | 30 +++ I looked through the incremental changes, and they look ok, but see that they do introduce new test failures: Oh of course - I'll add this new parameter to the RNG schema - nice to see this test doing its job preventing us adding new XML without corresponding schema updates :-) Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
On Fri, Jan 30, 2009 at 02:41:18PM +, Daniel P. Berrange wrote:
On Fri, Jan 30, 2009 at 03:27:55PM +0100, Jim Meyering wrote:
Daniel P. Berrange berra...@redhat.com wrote:
...
Further up in this code its already adding cache=off for shared disks.
Probably introduced by me in the merge.
There's also quite a few code style issues, not following conventions
of the surrounding code.
Here's a patch which addresses all that, and more importantly adds test
cases covering all the cache variants QEMU has.
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.args |
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-none.xml |
29 +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.args |
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wb.xml|
29 +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.args |
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v1-wt.xml|
29 +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.args |
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml |
29 +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.args |
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wb.xml|
29 +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.args |
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-wt.xml|
29 +++
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args|
1
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml |
30 +++
I looked through the incremental changes, and they look ok,
but see that they do introduce new test failures:
Oh of course - I'll add this new parameter to the RNG schema - nice
to see this test doing its job preventing us adding new XML without
corresponding schema updates :-)
Here's the patch including RNG schema update
Daniel
diff -r 2ff2ff7734c2 docs/schemas/domain.rng
--- a/docs/schemas/domain.rng Fri Jan 30 11:01:52 2009 +
+++ b/docs/schemas/domain.rng Fri Jan 30 14:47:11 2009 +
@@ -426,16 +426,43 @@
--
define name='driver'
element name='driver'
- attribute name='name'
+ choice
+ group
+ ref name='driverFormat'/
+ optional
+ ref name='driverCache'/
+ /optional
+ /group
+ group
+ optional
+ ref name='driverFormat'/
+ /optional
+ ref name='driverCache'/
+ /group
+ /choice
+ empty/
+/element
+ /define
+
+ define name='driverFormat'
+attribute name='name'
+ ref name='genericName'/
+/attribute
+optional
+ attribute name='type'
ref name='genericName'/
/attribute
- optional
-attribute name='type'
- ref name='genericName'/
- /attribute
- /optional
- empty/
-/element
+/optional
+ /define
+
+ define name='driverCache'
+attribute name='cache'
+ choice
+ valuenone/value
+ valuewriteback/value
+ valuewritethrough/value
+ /choice
+/attribute
/define
define name='filesystem'
diff -r 2ff2ff7734c2 src/domain_conf.c
--- a/src/domain_conf.c Fri Jan 30 11:01:52 2009 +
+++ b/src/domain_conf.c Fri Jan 30 14:47:11 2009 +
@@ -1,7 +1,7 @@
/*
* domain_conf.c: domain XML processing
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -91,6 +91,12 @@ VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMA
usb,
uml)
+VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
+ default,
+ none,
+ writethrough,
+ writeback);
+
VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
mount,
block,
@@ -568,6 +574,7 @@ virDomainDiskDefParseXML(virConnectPtr c
char *source = NULL;
char *target = NULL;
char *bus = NULL;
+char *cachetag = NULL;
if (VIR_ALLOC(def) 0) {
virReportOOMError(conn);
@@ -617,6 +624,7 @@ virDomainDiskDefParseXML(virConnectPtr c
(xmlStrEqual(cur-name, BAD_CAST driver))) {
driverName = virXMLPropString(cur, name);
driverType = virXMLPropString(cur, type);
+cachetag = virXMLPropString(cur, cache);
} else if (xmlStrEqual(cur-name, BAD_CAST readonly)) {
def-readonly = 1;
} else if (xmlStrEqual(cur-name, BAD_CAST shareable)) {
@@ -713,6 +721,13 @@ virDomainDiskDefParseXML(virConnectPtr c
goto error;
}
+if (cachetag
+(def-cachemode =
Re: [libvirt] Re: [PATCH 0/3] A small example program
Daniel P. Berrange berra...@redhat.com wrote:
On Fri, Jan 30, 2009 at 02:32:00PM +0100, Jim Meyering wrote:
Jim Meyering j...@meyering.net wrote:
I've just applied your 3 patches and built the example.
That went fine. Then I tried to run it like this:
...
$ kill $pid
$ LIBVIRT_DEBUG=1 ../../qemud/libvirtd 2 log
$ ./hellolibvirt
Attempting to connect to hypervisor
Connected to hypervisor at qemu:///session
? - libvir: error : Unknown failure
Failed to get hypervisor version
Disconnected from hypervisor
[Exit 1]
This is on F10 x86_64.
FYI, I poked around in the server to see what was going wrong.
qemudGetVersion calls qemudExtractVersion, which calls
virCapabilitiesDefaultGuestEmulator, which compares the
single guest cap and gets an arch mismatch:
Hmm, it should use the native arch - virExtractVersionInfo needs
fixing to call uname, and extract the native arch. Hardcoding
i386 was sufficient, when we mandated that 'qemu' was always
present, but now we allow either qemu or KVM, so we need to
always use native. NB, i3/4/586 should be mapped to i686.
I see you already had to do that once.
From 0131996b3cd819624259d6adcc5d968d6a0210b1 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Fri, 30 Jan 2009 15:41:39 +0100
Subject: [PATCH] fix qemud version reporting when qemu is not installed
* src/qemu_conf.c (uname_normalize): New function, factored out of...
(qemudBuildCommandLine): ...here. Use the new function.
(qemudExtractVersion): Use it here, rather than hard-coding i686.
---
src/qemu_conf.c | 27 ++-
1 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index b4ec733..972ea50 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -486,17 +486,33 @@ rewait:
return ret;
}
+static void
+uname_normalize (struct utsname *ut)
+{
+uname(ut);
+
+/* Map i386, i486, i586 to i686. */
+if (ut-machine[0] == 'i'
+ut-machine[1] != '\0'
+ut-machine[2] == '8'
+ut-machine[3] == '6'
+ut-machine[4] == '\0')
+ut-machine[1] = '6';
+}
+
int qemudExtractVersion(virConnectPtr conn,
struct qemud_driver *driver) {
const char *binary;
struct stat sb;
+struct utsname ut;
if (driver-qemuVersion 0)
return 0;
+uname_normalize(ut);
if ((binary = virCapabilitiesDefaultGuestEmulator(driver-caps,
hvm,
- i686,
+ ut.machine,
qemu)) == NULL)
return -1;
@@ -718,14 +734,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
char domid[50];
char *pidfile;
-uname(ut);
-
-/* Nasty hack make i?86 look like i686 to simplify next comparison */
-if (ut.machine[0] == 'i'
-ut.machine[2] == '8'
-ut.machine[3] == '6'
-!ut.machine[4])
-ut.machine[1] = '6';
+uname_normalize(ut);
virUUIDFormat(vm-def-uuid, uuid);
--
1.6.1.2.418.gd79e6
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
Daniel P. Berrange berra...@redhat.com wrote: Here's the patch including RNG schema update Daniel diff -r 2ff2ff7734c2 docs/schemas/domain.rng --- a/docs/schemas/domain.rng Fri Jan 30 11:01:52 2009 + +++ b/docs/schemas/domain.rng Fri Jan 30 14:47:11 2009 + @@ -426,16 +426,43 @@ -- define name='driver' element name='driver' - attribute name='name' + choice + group + ref name='driverFormat'/ + optional + ref name='driverCache'/ + /optional + /group + group + optional + ref name='driverFormat'/ + /optional + ref name='driverCache'/ + /group + /choice + empty/ +/element + /define + + define name='driverFormat' +attribute name='name' + ref name='genericName'/ +/attribute +optional + attribute name='type' ref name='genericName'/ /attribute - optional -attribute name='type' - ref name='genericName'/ - /attribute - /optional - empty/ -/element +/optional + /define + + define name='driverCache' +attribute name='cache' + choice + valuenone/value + valuewriteback/value + valuewritethrough/value + /choice +/attribute /define define name='filesystem' Yep. ACK. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] eliminate strerror from qemu_driver.c: use virReportSystemError instead
On Thu, Jan 29, 2009 at 09:54:59PM +0100, Jim Meyering wrote:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 36e12b2..8fd789d 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -89,31 +89,19 @@ static void qemuDriverUnlock(struct qemud_driver *driver)
static int qemudSetCloseExec(int fd) {
int flags;
-if ((flags = fcntl(fd, F_GETFD)) 0)
-goto error;
-flags |= FD_CLOEXEC;
-if ((fcntl(fd, F_SETFD, flags)) 0)
-goto error;
-return 0;
- error:
-qemudLog(QEMUD_ERR,
- %s, _(Failed to set close-on-exec file descriptor flag\n));
-return -1;
+return ((flags = fcntl(fd, F_GETFD)) 0
+ || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) 0
+? -1
+: 0);
}
static int qemudSetNonBlock(int fd) {
int flags;
-if ((flags = fcntl(fd, F_GETFL)) 0)
-goto error;
-flags |= O_NONBLOCK;
-if ((fcntl(fd, F_SETFL, flags)) 0)
-goto error;
-return 0;
- error:
-qemudLog(QEMUD_ERR,
- %s, _(Failed to set non-blocking file descriptor flag\n));
-return -1;
+return ((flags = fcntl(fd, F_GETFL)) 0
+ || fcntl(fd, F_SETFL, flags | O_NONBLOCK) 0
+? -1
+: 0);
}
You can actually just kill off the SetNonBlock method, since we
added one to util.h. We should probably do same for SetCloseExec
since I believe we have several copies of that with the sme
problem too.
@@ -854,8 +842,7 @@ static int qemudWaitForMonitor(virConnectPtr conn,
qemudFindCharDevicePTYs,
console, 3000);
if (close(logfd) 0)
-qemudLog(QEMUD_WARN, _(Unable to close logfile: %s\n),
- strerror(errno));
+virReportSystemError(NULL, errno, %s, _(Unable to close
logfile));
return ret;
}
If you report an error in this scenario then you must also ensure
a failure code is returned. This particular scenario is non-fatal
though, so I don't think we should be raising an error here at all.
I'd be inclined to just keep qemudLog, but with the raw errno value
as an int.
@@ -1134,30 +1121,30 @@ static int qemudStartVMDaemon(virConnectPtr conn,
tmp = progenv;
while (*tmp) {
if (safewrite(vm-logfile, *tmp, strlen(*tmp)) 0)
-qemudLog(QEMUD_WARN, _(Unable to write envv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write envv to logfile));
if (safewrite(vm-logfile, , 1) 0)
-qemudLog(QEMUD_WARN, _(Unable to write envv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write envv to logfile));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm-logfile, *tmp, strlen(*tmp)) 0)
-qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write argv to logfile));
if (safewrite(vm-logfile, , 1) 0)
-qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write argv to logfile));
tmp++;
}
if (safewrite(vm-logfile, \n, 1) 0)
-qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d: %s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write argv to logfile));
if ((pos = lseek(vm-logfile, 0, SEEK_END)) 0)
-qemudLog(QEMUD_WARN, _(Unable to seek to end of logfile %d: %s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to seek to end of logfile));
Likewise with all these - we're just printing ARGV to a logfile - this should
not result in errors propagated to the caller - unless we intend to tear down
the VM we just started, but I think that's overkill.
@@ -1222,14 +1210,14 @@ static void qemudShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (virKillProcess(vm-pid, 0) == 0
virKillProcess(vm-pid, SIGTERM) 0)
-qemudLog(QEMUD_ERROR, _(Failed to send SIGTERM to %s (%d): %s\n),
- vm-def-name, vm-pid, strerror(errno));
+virReportSystemError(conn, errno,
+ _(Failed to send SIGTERM to %s (%d)),
+ vm-def-name, vm-pid);
virEventRemoveHandle(vm-monitor_watch);
if (close(vm-logfile) 0)
-
Re: [libvirt] Re: [PATCH 0/3] A small example program
On Fri, Jan 30, 2009 at 03:54:17PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: Hmm, it should use the native arch - virExtractVersionInfo needs fixing to call uname, and extract the native arch. Hardcoding i386 was sufficient, when we mandated that 'qemu' was always present, but now we allow either qemu or KVM, so we need to always use native. NB, i3/4/586 should be mapped to i686. I see you already had to do that once. Oh yes, forgot about that. From 0131996b3cd819624259d6adcc5d968d6a0210b1 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 30 Jan 2009 15:41:39 +0100 Subject: [PATCH] fix qemud version reporting when qemu is not installed * src/qemu_conf.c (uname_normalize): New function, factored out of... (qemudBuildCommandLine): ...here. Use the new function. (qemudExtractVersion): Use it here, rather than hard-coding i686. ACK to this. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
Jim Meyering wrote:
Hi John,
I tried to apply that, but failed miserably,
since all of the following was recently redone to
use virBufferVSprintf rather than snprintf.
Yea I suspected the code was likely seeing some
motion. Thanks for bringing it forward.
And it's
a good thing, because with the changes below, there was
potential buffer overrun: nc could end up larger than PATH_MAX.
You are correct. I mistook the return value of
snprintf() in the case of truncation.
One remaining bit that I haven't done:
add tests for this, exercising e.g.,
- cachemode
- !cachemode (disk-shared !disk-readonly)
- !cachemode (!disk-shared || disk-readonly)
That logic should be correct as it exists in the
patch, namely we'll generate a cache=* flag in
the case cachemode was specified explicitly in
the xml definition or for the preexisting case of
(shared !readonly). Here if both happen to be
true the explicit xml cache tag takes precedence.
But with the existing code we need to remove the
clause of:
@@ -1007,18 +1025,34 @@ int qemudBuildCommandLine(virConnectPtr
if (bootable
disk-device == VIR_DOMAIN_DISK_DEVICE_DISK)
virBufferAddLit(opt, ,boot=on);
- if (disk-shared !disk-readonly)
- virBufferAddLit(opt, ,cache=off);
if (disk-driverType)
virBufferVSprintf(opt, ,fmt=%s, disk-driverType);
As this results in generation of a cache=*
twice on the qemu cmdline, and possibly of
different dispositions.
With this change I think it is good to go.
The attached patch incorporates the above
modification.
-john
--
john.coo...@redhat.com
domain_conf.c | 34 ++
domain_conf.h | 16
qemu_conf.c | 43 ++-
qemu_conf.h |3 ++-
4 files changed, 82 insertions(+), 14 deletions(-)
=
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -81,6 +81,21 @@ enum virDomainDiskBus {
VIR_DOMAIN_DISK_BUS_LAST
};
+/* summary of all possible host open file cache modes
+ */
+typedef enum {
+VIR_DOMAIN_DISK_CACHE_UNSPECIFIED, /* reserved */
+VIR_DOMAIN_DISK_CACHE_OFF,
+VIR_DOMAIN_DISK_CACHE_ON,
+VIR_DOMAIN_DISK_CACHE_DISABLE,
+VIR_DOMAIN_DISK_CACHE_WRITEBACK,
+VIR_DOMAIN_DISK_CACHE_WRITETHRU,
+
+VIR_DOMAIN_DISK_CACHE_LAST
+} virDomainDiskCache;
+
+VIR_ENUM_DECL(virDomainDiskCache)
+
/* Stores the virtual disk configuration */
typedef struct _virDomainDiskDef virDomainDiskDef;
typedef virDomainDiskDef *virDomainDiskDefPtr;
@@ -92,6 +107,7 @@ struct _virDomainDiskDef {
char *dst;
char *driverName;
char *driverType;
+int cachemode;
unsigned int readonly : 1;
unsigned int shared : 1;
int slotnum; /* pci slot number for unattach */
=
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1,7 +1,7 @@
/*
* config.c: VM configuration management
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -399,7 +399,9 @@ int qemudExtractVersionInfo(const char *
if (strstr(help, -domid))
flags |= QEMUD_CMD_FLAG_DOMID;
if (strstr(help, -drive))
-flags |= QEMUD_CMD_FLAG_DRIVE;
+flags |= QEMUD_CMD_FLAG_DRIVE |
+(strstr(help, cache=writethrough|writeback|none) ?
+QEMUD_CMD_FLAG_DRIVE_CACHEMODE : 0);
if (strstr(help, boot=on))
flags |= QEMUD_CMD_FLAG_DRIVE_BOOT;
if (version = 9000)
@@ -591,6 +593,22 @@ qemudNetworkIfaceConnect(virConnectPtr c
return NULL;
}
+VIR_ENUM_DECL(qemu_cache_map)
+
+/* map from internal cache mode to qemu cache arg text
+ *
+ * Note: currently this replicates virDomainDiskCache, but will need to
+ * error flag potential new entries in virDomainDiskCache which are
+ * not supported by qemu (raising exceptions as appropriate).
+ */
+VIR_ENUM_IMPL(qemu_cache_map, VIR_DOMAIN_DISK_CACHE_LAST,
+, /* reserved -- mode not specified */
+off, /* deprecated; use none */
+on, /* obsolete; use writethrough */
+none,
+writeback,
+writethrough)
+
static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev,
char *buf,
int buflen)
@@ -1007,11 +1025,27 @@ int qemudBuildCommandLine(virConnectPtr
if (bootable
disk-device == VIR_DOMAIN_DISK_DEVICE_DISK)
virBufferAddLit(opt, ,boot=on);
-if (disk-shared !disk-readonly)
-virBufferAddLit(opt, ,cache=off);
if (disk-driverType)
virBufferVSprintf(opt, ,fmt=%s, disk-driverType);
+unsigned int qf;
+int cachemode = disk-cachemode;
+if (cachemode) {
+if
Re: [libvirt] PATCH: Fix infinite loop when QEMU quits at startup
On Fri, Jan 30, 2009 at 11:37:35AM +, Daniel P. Berrange wrote: diff -r 826e6ed70ee0 src/qemu_driver.c --- a/src/qemu_driver.c Fri Jan 30 10:58:34 2009 + +++ b/src/qemu_driver.c Fri Jan 30 11:00:43 2009 + @@ -355,10 +355,9 @@ qemudReconnectVMs(struct qemud_driver *d qemudLog(QEMUD_ERR, _(Failed to reconnect monitor for %s: %d\n), vm-def-name, rc); goto next_error; -} else -vm-monitorpath = status-monitorpath; Don't we loose initializing vm-monitorpath on reconnect this way? The rest of the patch looks fine to me. -- Guido - -if((vm-logfile = qemudLogFD(NULL, driver-logDir, vm-def-name)) 0) +} + +if ((vm-logfile = qemudLogFD(NULL, driver-logDir, vm-def-name)) 0) return -1; -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] (resend) Problems with virt-manager checking access on virtual images.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: On Fri, Jan 30, 2009 at 09:06:38AM -0500, Daniel J Walsh wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Daniel P. Berrange wrote: I don't particularly like the idea of running another program to check this because SELinux context isn't the only thing which will potentially prevent QEMU accessing the disk image. CGroups device policy make prevent it. A setuid() call in QEMU to drop privileges, may prevent it. It may be asked to open it read-write, and only be able to open it read-only . It may want to take a lock on the image, but it already be locked, and so on. QEMU does already report pretty much the same error message as the qemuaccess program does when it is unable to access a disk image # virsh start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso IMHO, this all stems from a mistake I made when designing the original virt-manager UI. Namely, I should never have used a generic file dialog which allows selection of disk images anywhere on the host. It was wrong for general disk images, in just the same way that its wrong for ISO images. And absolutely useless for remote provisioning. With the storage pool APIs we have the opportunity to fix this in a way helps not only the SELinux case, but also the app in general. Instead of showing a generic file dialog, we should just show a dialog displaying the configured storage pools, and allow the user to pick an ISO out of one of the the pools. virt-manager should offer to remember which storage pool contains installable ISO images, and which should be used by default for disk images. We already have /var/lib/libvirt/images by default for disk images, and should add /var/lib/libvirt/isos too. If a user downloads an ISO to their home directory, we could easily have a option in the UI for 'Import an ISO file to the pool' which would move it into the correct location. NB, here I am talking about use of the system-wide QEMU driver instance of 'qemu:///system', which runs privileged on the host. This is really intended at server deployments of virt, but we have been happening to use it for general ad-hoc desktop usage too in Fedora. There is also a per-user 'qemu://session' UI where both libvirtd and the QEMU guests run unprivileged as that user's UID. In that scenario the storage pools should use something like $HOME/VirtualMachines/images and $HOME/VirtualMachines/ISOs'as the default pools for disk ISOs respectively. I would like to see Fedora use qemu:///session by default for generic desktop virt usage, in which case everything runs as that UID, and keeps everything in $HOME. We discussed this also. The only problem I see here is the additional disk usage, since you would permanently keeping the iso, even though you might only be using it once. What about removable media, cdrom and usb key isos. Do you want to copy them to the /var/lib/libvirt/isos directory also? If think if someone downloads a large ISO its fairly likely they'll want to keep it around for some amount of time, rather than risk having to download it again. That said, our storage pools APIs can trivally be used to delete it if it is no longer required. For removable media, we're just directly attaching the block device, so we can't avoid the need to re-label from fixed_dev_t (or equiva) to virt_image_t - though having a virt_image_ro_t for readonly access would be preferrable for cases where we're explicitly attaching it to the guest in read-only mode. USB keys I'm not so sure about - in the common case they'll be FAT filesystem which doesn't support labelling at all. So we'd either have to ensure its mounted with a suitable context= mount option, or get the policy to to allow read-only access to the default mount context for FAT filesystems. The /var/lib/libvirt/isos directly is just a plain directory based storage pool. We also have concept of filesystem based storage pools (eg a local device, mounted in some directory) which can we directly manage. Likewise NFS mounts we can handle directly, mounting a remote directoy from a server in a specific place, so we can ensure a context= mount option is used. So I think a combination of factors - Default directories for local host stored file based images ISO which get correct context automatically - libvirt has to set contenxt on any block device nodes which need to be assigned - Setting mount context for removable disks / changing policy to allow read-only access to mounted removable disks. Are the isos in this directory going to be Read Only or can some qemus read/write them? Any files used to back the QEMU CDROM device should be restricted to be read-only, since we need ability to safely assign the same ISO to multiple guests concurrently - for concurrent
Re: [libvirt] [PATCH] Fix getpwuid_r() usage
On Thu, Jan 29, 2009 at 06:06:10PM -0800, john.le...@sun.com wrote:
@@ -1485,7 +1485,14 @@ char *virGetUserDirectory(virConnectPtr
return NULL;
}
-if (getpwuid_r(uid, pwbuf, strbuf, strbuflen, pw) != 0) {
+/*
+ * From the manpage (terrifying but true):
+ *
+ * ERRORS
+ * 0 or ENOENT or ESRCH or EBADF or EPERM or ...
+ *The given name or uid was not found.
+ */
+if (getpwuid_r(uid, pwbuf, strbuf, strbuflen, pw) != 0 || pw == NULL)
virReportSystemError(conn, errno,
_(Failed to find user record for uid '%d'),
uid);
Argh, I only read as far as
The getpwnam_r() and getpwuid_r() functions return zero on success.
In case of error, and error number is returned
So it returns 0 on success, except when 0 means failure :-(
ACK to your patch
Daniel
--
|: 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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix yet another printf(%s, NULL) case
On Thu, Jan 29, 2009 at 06:34:51PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1233282885 28800 # Node ID 6ee939f57a02bf9d332f094e07180e0149e85924 # Parent c60439e564f90b579c07f6349f8f0810a5da1032 Fix yet another printf(%s, NULL) case Signed-off-by: John Levon john.le...@sun.com diff --git a/src/libvirt.c b/src/libvirt.c --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2691,7 +2691,7 @@ virDomainMigrate (virDomainPtr domain, char *dom_xml = NULL; int cookielen = 0, ret, version = 0; DEBUG(domain=%p, dconn=%p, flags=%lu, dname=%s, uri=%s, bandwidth=%lu, - domain, dconn, flags, dname, uri, bandwidth); + domain, dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); ACK Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix virsh migrateuri handling
On Thu, Jan 29, 2009 at 07:37:25PM -0800, john.le...@sun.com wrote: # HG changeset patch # User john.le...@sun.com # Date 1233286638 28800 # Node ID 0934f4c2e1d446b1902d9ffcf14febf964af1e6a # Parent 6ee939f57a02bf9d332f094e07180e0149e85924 Fix virsh migrateuri handling Signed-off-by: John Levon john.le...@sun.com diff --git a/src/virsh.c b/src/virsh.c --- a/src/virsh.c +++ b/src/virsh.c @@ -2254,11 +2254,9 @@ cmdMigrate (vshControl *ctl, const vshCm goto done; } -migrateuri = vshCommandOptString (cmd, migrateuri, found); -if (!found) migrateuri = NULL; - -dname = vshCommandOptString (cmd, dname, found); -if (!found) migrateuri = dname; +migrateuri = vshCommandOptString (cmd, migrateuri, NULL); + +dname = vshCommandOptString (cmd, dname, NULL); Urgh, took me a while to understand what was going wrong there - a little bizzarre contract for vshCommandOptString! ACK to this Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix virsh migrateuri handling
On Fri, Jan 30, 2009 at 03:25:36PM +, Daniel P. Berrange wrote: Urgh, took me a while to understand what was going wrong there - a little bizzarre contract for vshCommandOptString! Yes, I'm completely lost as to the purpose of found to be honest. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration API broken for xenmigr://
On Thu, Jan 29, 2009 at 10:50:18PM -0500, John Levon wrote: 2675 * Returns the new domain object if the migration was successful, 2676 * or NULL in case of error. Note that the new domain object 2677 * exists in the scope of the destination connection (dconn). This is obviously impossible for xenmigr:///. As a result, virsh migrate always returns an error code: I'm not sure I understand why this is impossible ? You always have two virConnectPtr objects - one of the source, and one for the destination host. The reason for the optional URI is to cope with the scenario where the clients view of what the destination hostname is, has to be different from the source host's view. eg, if a node is multi-homed, 10.0.0.1 and 192.168.1.1 Then the libvirt client might connect to source and estination hosts with xen://10.0.0.1/ (source) xen://10.0.0.2/ (dest) But, the source dest may only be able to talk to each other using 192.168.1.1/.2, so yu'd have to give a extra URI for xenmigr://192.168.1.2/ so the source host used the correct network address for the actual migration. Either way, the libvirt client can get the resulting domain object, from the destination hosts virConnectPtr instance # virsh migrate --live domu-220 xen:/// interpol 18:44:40.974: error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName thewhip:xend # echo $? 1 Since virDomainLookupByName() returns NULL. I will request again, can I remove the 'error' code from xenUnifiedDomainLookupByName? I'm not sure why it returns NULL there ?!?! More importantly, how can this be fixed? Perhaps we can return 'domain' instead of 'ddomain' if a migrateuri was specified? Also, where does 'domain' get freed in the case where 'ddomain' is returned? The passed in 'domain' object is owned by the caller, so the caller is responsible for free'ing it once its done using it. Likewise the returned ddomain object should also be free'd by the caller when it no longer requires it Danoel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Fix leak in libvirtd daemon when reporting errors
Daniel Veillard veill...@redhat.com wrote: On Fri, Jan 30, 2009 at 11:44:35AM +, Daniel P. Berrange wrote: When reporting libvirt errors back to the client, we forgot to free the memory associated with the remote_error object - principally the char * message strings. So every error reported would leak some memory. This patch free's the memory Sounds a fairly important fix, +1 Yeah, really. ACK -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Fix leak in storage driver
On Fri, Jan 30, 2009 at 04:55:40PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: A recent change to keep the storage pools active upon shutdown, exposed a minor flaw in the code which free's a virStoragePoolObj instance. It never free's the associated volumes, since it presumed you'd never free a pool, which was still active. A bogus assumption, causing us to leak memory upon daemon shutdown, thus annoying valgrind. Daniel diff -r 3e95abd6df89 src/storage_conf.c --- a/src/storage_conf.cFri Jan 30 11:01:10 2009 + +++ b/src/storage_conf.cFri Jan 30 11:01:29 2009 + @@ -296,6 +296,8 @@ virStoragePoolObjFree(virStoragePoolObjP if (!obj) return; +virStoragePoolObjClearVols(obj); + ACK. This is crying for integration testing... This scenario really ought to have been caught by tests of the storage impl in test:///default driver - it could be that our impl in the test driver is not complete enough. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration API broken for xenmigr://
On Fri, Jan 30, 2009 at 03:41:04PM +, Daniel P. Berrange wrote: This is obviously impossible for xenmigr:///. As a result, virsh migrate always returns an error code: I'm not sure I understand why this is impossible ? I thought the purpose of xenmigr:/// was to allow live migration the traditional way without needing a remotely accessible libvirtd. At least, that's how we're using it. If it's the intention of libvirt that you must always have a listening remote API, then we'll have to work out a private fix for our tree. # virsh migrate --live domu-220 xen:/// interpol 18:44:40.974: error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName thewhip:xend # echo $? 1 Since virDomainLookupByName() returns NULL. I will request again, can I remove the 'error' code from xenUnifiedDomainLookupByName? I'm not sure why it returns NULL there ?!?! xen:/// (the local source host) no longer has the domain, since it moved to interpol. More importantly, how can this be fixed? Perhaps we can return 'domain' instead of 'ddomain' if a migrateuri was specified? Also, where does 'domain' get freed in the case where 'ddomain' is returned? The passed in 'domain' object is owned by the caller, so the caller is responsible for free'ing it once its done using it. Likewise the returned ddomain object should also be free'd by the caller when it no longer requires it Yuck, so that won't do. Perhaps we could duplicate domain and pass that back. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration API broken for xenmigr://
On Fri, Jan 30, 2009 at 11:04:12AM -0500, John Levon wrote: On Fri, Jan 30, 2009 at 03:41:04PM +, Daniel P. Berrange wrote: This is obviously impossible for xenmigr:///. As a result, virsh migrate always returns an error code: I'm not sure I understand why this is impossible ? I thought the purpose of xenmigr:/// was to allow live migration the traditional way without needing a remotely accessible libvirtd. At least, that's how we're using it. If it's the intention of libvirt that you must always have a listening remote API, then we'll have to work out a private fix for our tree. # virsh migrate --live domu-220 xen:/// interpol 18:44:40.974: error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName thewhip:xend # echo $? 1 Since virDomainLookupByName() returns NULL. I will request again, can I remove the 'error' code from xenUnifiedDomainLookupByName? I'm not sure why it returns NULL there ?!?! xen:/// (the local source host) no longer has the domain, since it moved to interpol. Yeah, so you've passed invalid data to the migrate call by giving it two connections which both point to the source host. Would should really add a sanity check to try detect that mistake. Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH 0/3] A small example program
Daniel P. Berrange wrote: On Fri, Jan 30, 2009 at 02:10:58PM +0100, Jim Meyering wrote: Hi David, I've just applied your 3 patches and built the example. That went fine. Then I tried to run it like this: $ LIBVIRT_DEBUG=1 libvirtd 2 log pid=$! $ ./hellolibvirt Attempting to connect to hypervisor Connected to hypervisor at qemu:///session ? - Failed to get hypervisor version This shows a place where the example program could use the libvirt error information - it should have received error message to the effect of 'Cannot find QEMU binary /usr/bin/qemu' when qemudExtractVersion failed to find the binary it wanted I should illustrate getting the error messages in the example. I'll put it in. Did you deliberately break your system to provoke the failure? I had to rename /usr/bin/qemu to get it. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH 0/3] A small example program
On Fri, Jan 30, 2009 at 11:23:29AM -0500, Dave Allan wrote: Daniel P. Berrange wrote: On Fri, Jan 30, 2009 at 02:10:58PM +0100, Jim Meyering wrote: Hi David, I've just applied your 3 patches and built the example. That went fine. Then I tried to run it like this: $ LIBVIRT_DEBUG=1 libvirtd 2 log pid=$! $ ./hellolibvirt Attempting to connect to hypervisor Connected to hypervisor at qemu:///session ? - Failed to get hypervisor version This shows a place where the example program could use the libvirt error information - it should have received error message to the effect of 'Cannot find QEMU binary /usr/bin/qemu' when qemudExtractVersion failed to find the binary it wanted I should illustrate getting the error messages in the example. I'll put it in. Did you deliberately break your system to provoke the failure? I had to rename /usr/bin/qemu to get it. Just don't install QEMU package - just have KVM Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration API broken for xenmigr://
On Fri, Jan 30, 2009 at 04:19:06PM +, Daniel P. Berrange wrote: I'm not sure I understand why this is impossible ? I thought the purpose of xenmigr:/// was to allow live migration the traditional way without needing a remotely accessible libvirtd. At least, that's how we're using it. If it's the intention of libvirt that you must always have a listening remote API, then we'll have to work out a private fix for our tree. Could you answer this bit? It sounds like from your other comment that the above is true. We're not going to *require* libvirtd to listen across the network when there's an existing mechanism, so it sounds like we'll have to fork this part. regards, john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration API broken for xenmigr://
On Fri, Jan 30, 2009 at 11:36:16AM -0500, John Levon wrote: On Fri, Jan 30, 2009 at 04:19:06PM +, Daniel P. Berrange wrote: I'm not sure I understand why this is impossible ? I thought the purpose of xenmigr:/// was to allow live migration the traditional way without needing a remotely accessible libvirtd. At least, that's how we're using it. If it's the intention of libvirt that you must always have a listening remote API, then we'll have to work out a private fix for our tree. Could you answer this bit? It sounds like from your other comment that the above is true. Yes, the migrate API requires a connection to both libvirtd daemons, source and destination. We're not going to *require* libvirtd to listen across the network when there's an existing mechanism, so it sounds like we'll have to fork this part. You can use the xen+ssh://hostname/ URI for the destination if you don't want libvirtd to listen publically Daniel -- |: 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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration API broken for xenmigr://
On Fri, Jan 30, 2009 at 04:42:04PM +, Daniel P. Berrange wrote: We're not going to *require* libvirtd to listen across the network when there's an existing mechanism, so it sounds like we'll have to fork this part. You can use the xen+ssh://hostname/ URI for the destination if you don't want libvirtd to listen publically Which requires setting up ssh keys and is significantly slower. Again, we're not going forbid use of an existing mechanism, but we definitely don't want to make people use xm, so we'll just hack it in. regards, john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Fix infinite loop when QEMU quits at startup
Daniel P. Berrange berra...@redhat.com wrote: The recent refactoring of the QEMU startup process now reads the monitor TTY from the logfile. Unfortunately in this refactoring we lost the check for the 'ret == 0' scenario in the read() return value. So if QEMU quits at startup, eg due to missing disk image, we loop forever on read() == 0 because we hit end-of-file and QEMU has quit. This patch adds back handling for this scenario, and takes care to propagate the contents of the log to the user as an error message # start demo libvir: QEMU error : internal error unable to start guest: qemu: could not open disk image /home/berrange/Fedora-9-i686-Live.iso error: Failed to start domain demo In addition, there were a couple of other bugs - a memory leak where we set the 'monitorpath' variable, even though we'd just set it moments before. - a missing check for whether the driver VNC password was present when initializing passwords at VM startupo - missing initialization of the monitor_watch field, and missing checking for whether the watch was set before removing it. - a gratuitous LOG_INFO when shutting down any VM, which could just be LOG_DEBUG. FYI, I reproduced the infloop with the following: cat \EOF d.xml domain type='qemu' nameD/name uuidc7a5fdbd-cdaf-9455-926a-d65c16db1809/uuid memory219200/memory currentMemory219200/currentMemory vcpu2/vcpu os type arch='i686' machine='pc'hvm/type boot dev='cdrom'/ /os devices emulator/usr/bin/qemu-system-x86_64/emulator disk type='file' device='cdrom' source file='no-such-file'/ target dev='hdc'/ readonly/ /disk disk type='file' device='disk' source file='no-such-file'/ target dev='hda'/ /disk graphics type='vnc' port='-1'/ /devices /domain EOF Before, this use of virsh would hang: qemud/libvirtd src/virsh -c qemu:///session define d.xml; start D Now, it fails with a diagnostic, as you'd expect. Somehow, while testing this, I got numerous segfaults from libvirtd (due to dereferencing NULL doms-objs[i]-def, but those should never be NULL), yet when I went to set up a reproducer, it stopped happening altogether. So not only does the infloop-fix look like it does the right thing, I confirmed it with the above. The rest looks fine, too. Once we have something like my unix_sock_dir patch or an improved testing framework, I'll add a test like the above. Or maybe you know how to reproduce the infloop using the test driver? ACK. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] failed test on Fedora 8
17) QEMU XML-2-ARGV disk-drive-shared ... FAILED I have VIR_TEST_DEBUG set, but this is all I get. It's new regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Thu, 2009-01-29 at 13:34 -0800, Kaitlin Rupert wrote: Mark McLoughlin wrote: I don't think we want to define a bridge here, but more that an interface is shared - i.e. this is a property of eth2. Note this line. The main concern is that this is the way I'd expect NetworkManager to support it - i.e. that you could configure NetworkManager to share eth0, rather than ask it to create br0 and add eth0 to it. If you just want to create a bridge, you can creati a virtual network. Sorry to chime in so late... the virtual network support only allows the user to define bridges with NAT/routed forwarding. I tripped over this the first time, too; to put words in Mark's mouth, he meant 'bridge' here in the sense of a 'vritual network'. He calls a bridge with one enslaved physical NIC a 'shared network interface' (and doesn't much care for bridges with more NIC's ;) After talking to NM people, there doesn't seem to be a concern with NM compat - they are fine with the more explicit representation of a bridge. Would the still behave as a virtual network pool in this case? If multiple guests are tied to the same bridge, it would be useful to represent this as some kind of pool or grouping. Not sure what you mean here ... with all this, you'd still set your guests up the way you do today, with appropriate sources for their network interfaces, e.g. source bridge='br0'/ David -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix empty declaration compiler error
# HG changeset patch # User John Levon john.le...@sun.com # Date 129660 28800 # Node ID 69992f43b6f634fa46c7ae2039666378ce378eea # Parent 1b00b26880ad7feae71bb8df9ae86cfcaa428458 Fix empty declaration compiler error Signed-off-by: John Levon john.le...@sun.com diff --git a/src/domain_conf.c b/src/domain_conf.c --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -95,7 +95,7 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DO default, none, writethrough, - writeback); + writeback) VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, mount, -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] autostart with xend
It doesn't work quite right with a running domain. The 'new' will correctly set the config, but the temporary config doesn't include the new setting. So tests of the form set autostart; is it set? will fail. Suggestions? Also, why isn't this represented in the XML? thanks, john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] failed test on Fedora 8
Daniel P. Berrange berra...@redhat.com wrote:
From 560e27e1576a4c0ebe7db3e697ed9b6d8aa88fbc Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Fri, 30 Jan 2009 20:06:48 +0100
Subject: [PATCH] tests: diagnose open failure
* tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Diagnose
failure to open an input file.
---
tests/qemuxml2argvtest.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 90b4740..1d7aeb9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -36,8 +36,10 @@ static int testCompareXMLToArgvFiles(const char *xml,
virDomainDefPtr vmdef = NULL;
virDomainObj vm;
-if (virtTestLoadFile(cmd, expectargv, MAX_FILE) 0)
+if (virtTestLoadFile(cmd, expectargv, MAX_FILE) 0) {
+fprintf(stderr, failed to open %s: %s\n, cmd, strerror (errno));
goto fail;
+}
if (!(vmdef = virDomainDefParseFile(NULL, driver.caps, xml,
VIR_DOMAIN_XML_INACTIVE)))
ACK
Applied.
But as John Levon pointed out, there are many more tests/*test.c
programs that use virtTestLoadFile in exactly the same way:
$ git grep -h 'if (virtTestLoadFile'
if (virtTestLoadFile(outputfile, expect, MAX_FILE) 0)
if (virtTestLoadFile(cmd, expectargv, MAX_FILE) 0) {
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(sexpr, sexprPtr, MAX_FILE) 0)
if (virtTestLoadFile(expect, expectPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, expectxml, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(xmcfg, xmcfgPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(xmcfg, xmcfgPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(sexpr, sexprPtr, MAX_FILE) 0)
So rather than applying the same multi-line fix to all of those uses,
I expect to revert that patch and move the error reporting
into virtTestLoadFile itself.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] failed test on Fedora 8
On Fri, Jan 30, 2009 at 11:06:48PM +0100, Jim Meyering wrote:
But as John Levon pointed out, there are many more tests/*test.c
programs that use virtTestLoadFile in exactly the same way:
$ git grep -h 'if (virtTestLoadFile'
if (virtTestLoadFile(outputfile, expect, MAX_FILE) 0)
if (virtTestLoadFile(cmd, expectargv, MAX_FILE) 0) {
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(sexpr, sexprPtr, MAX_FILE) 0)
if (virtTestLoadFile(expect, expectPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, expectxml, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(xmcfg, xmcfgPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(xmcfg, xmcfgPtr, MAX_FILE) 0)
if (virtTestLoadFile(xml, xmlPtr, MAX_FILE) 0)
if (virtTestLoadFile(sexpr, sexprPtr, MAX_FILE) 0)
So rather than applying the same multi-line fix to all of those uses,
I expect to revert that patch and move the error reporting
into virtTestLoadFile itself.
Seems like a sensible idea to me.
Daniel
--
|: 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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] eliminate strerror from qemu_driver.c: use virReportSystemError instead
Daniel P. Berrange berra...@redhat.com wrote:
On Thu, Jan 29, 2009 at 09:54:59PM +0100, Jim Meyering wrote:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
You can actually just kill off the SetNonBlock method, since we
added one to util.h. We should probably do same for SetCloseExec
since I believe we have several copies of that with the sme
problem too.
Ah. I see that now. Have done both things.
@@ -854,8 +842,7 @@ static int qemudWaitForMonitor(virConnectPtr conn,
qemudFindCharDevicePTYs,
console, 3000);
if (close(logfd) 0)
-qemudLog(QEMUD_WARN, _(Unable to close logfile: %s\n),
- strerror(errno));
+virReportSystemError(NULL, errno, %s, _(Unable to close
logfile));
return ret;
}
If you report an error in this scenario then you must also ensure
a failure code is returned. This particular scenario is non-fatal
Why? because of semantics implied by the Error suffix?
Then maybe we need a *Warn function with similar functionality.
though, so I don't think we should be raising an error here at all.
I'd be inclined to just keep qemudLog, but with the raw errno value
as an int.
Why not? If I get EIO or ENOSPC, I'd like to see diagnostics
in the logs ASAP. The closer to the point of origin the better.
If we keep using qemudLog, then I'll just make it use virStrerror or
something along those lines. there's no need to print raw errno values.
@@ -1134,30 +1121,30 @@ static int qemudStartVMDaemon(virConnectPtr conn,
tmp = progenv;
while (*tmp) {
if (safewrite(vm-logfile, *tmp, strlen(*tmp)) 0)
-qemudLog(QEMUD_WARN, _(Unable to write envv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write envv to
logfile));
if (safewrite(vm-logfile, , 1) 0)
-qemudLog(QEMUD_WARN, _(Unable to write envv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write envv to
logfile));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm-logfile, *tmp, strlen(*tmp)) 0)
-qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write argv to
logfile));
if (safewrite(vm-logfile, , 1) 0)
-qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d:
%s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write argv to
logfile));
tmp++;
}
if (safewrite(vm-logfile, \n, 1) 0)
-qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d: %s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to write argv to logfile));
if ((pos = lseek(vm-logfile, 0, SEEK_END)) 0)
-qemudLog(QEMUD_WARN, _(Unable to seek to end of logfile %d: %s\n),
- errno, strerror(errno));
+virReportSystemError(NULL, errno,
+ %s, _(Unable to seek to end of logfile));
Likewise with all these - we're just printing ARGV to a logfile - this should
not result in errors propagated to the caller - unless we intend to tear down
the VM we just started, but I think that's overkill.
But failure still means write errors. It doesn't matter
so much what we're writing as *that* there's been a relatively
serious (write) error. Not reporting a write error now could
easily mask or delay reporting a later one involving important data.
That said, I don't feel very strongly either way,
so tell me what you'd prefer.
Hmm, these Log - Error conversions all are same non-fatal scneario
I mention earlier.
@@ -1492,7 +1480,7 @@ static int kvmGetMaxVCPUs(void) {
fd = open(KVM_DEVICE, O_RDONLY);
if (fd 0) {
-qemudLog(QEMUD_WARN, _(Unable to open %s: %s\n), KVM_DEVICE,
strerror(errno));
+virReportSystemError(NULL, errno, _(Unable to open %s),
KVM_DEVICE);
return maxvcpus;
}
This conversion looks good - we SHOULD be raising a real error
here. The original code was wrong to return 'maxvcpus' here,
it should be -1 upon error.
I've just converted another that I'll merge onto the rest:
From d02669442e0cd2354e6b429dbedcf665d15cfa87 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Fri, 30 Jan 2009 23:18:04 +0100
Subject: [PATCH] report OOMError
---
src/qemu_driver.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/qemu_driver.c
[libvirt] [PATCH 0/1] Add error reporting to example
The following patch adds error reporting to the example code. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add showError function to display error data generated by libvirt.
---
examples/hellolibvirt/hellolibvirt.c | 46 ++
1 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c
b/examples/hellolibvirt/hellolibvirt.c
index cc1af0f..234637e 100644
--- a/examples/hellolibvirt/hellolibvirt.c
+++ b/examples/hellolibvirt/hellolibvirt.c
@@ -6,6 +6,43 @@
#include stdio.h
#include stdlib.h
#include libvirt/libvirt.h
+#include libvirt/virterror.h
+
+static void
+showError(virConnectPtr conn)
+{
+int ret;
+virErrorPtr err;
+
+err = malloc(sizeof(*err));
+if (NULL == err) {
+printf(Could not allocate memory for error data\n);
+goto out;
+}
+
+ret = virConnCopyLastError(conn, err);
+
+switch (ret) {
+case 0:
+printf(No error found\n);
+break;
+
+case -1:
+printf(Parameter error when attempting to get last error\n);
+break;
+
+default:
+printf(libvirt reported: \%s\\n, err-message);
+break;
+}
+
+virResetError(err);
+free(err);
+
+out:
+return;
+}
+
static int
showHypervisorInfo(virConnectPtr conn)
@@ -22,12 +59,14 @@ showHypervisorInfo(virConnectPtr conn)
if (NULL == hvType) {
ret = 1;
printf(Failed to get hypervisor type\n);
+showError(conn);
goto out;
}
if (0 != virConnectGetVersion(conn, hvVer)) {
ret = 1;
printf(Failed to get hypervisor version\n);
+showError(conn);
goto out;
}
@@ -57,6 +96,7 @@ showDomains(virConnectPtr conn)
if (-1 == numActiveDomains) {
ret = 1;
printf(Failed to get number of active domains\n);
+showError(conn);
goto out;
}
@@ -64,6 +104,7 @@ showDomains(virConnectPtr conn)
if (-1 == numInactiveDomains) {
ret = 1;
printf(Failed to get number of inactive domains\n);
+showError(conn);
goto out;
}
@@ -85,6 +126,7 @@ showDomains(virConnectPtr conn)
if (-1 == numNames) {
ret = 1;
printf(Could not get list of defined domains from hypervisor\n);
+showError(conn);
goto out;
}
@@ -124,6 +166,7 @@ main(int argc, char *argv[])
if (NULL == conn) {
ret = 1;
printf(No connection to hypervisor\n);
+showError(conn);
goto out;
}
@@ -131,6 +174,7 @@ main(int argc, char *argv[])
if (NULL == uri) {
ret = 1;
printf(Failed to get URI for hypervisor connection\n);
+showError(conn);
goto disconnect;
}
@@ -150,6 +194,8 @@ main(int argc, char *argv[])
disconnect:
if (0 != virConnectClose(conn)) {
printf(Failed to disconnect from hypervisor\n);
+showError(conn);
+ret = 1;
} else {
printf(Disconnected from hypervisor\n);
}
--
1.6.0.6
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
David Lutterkort wrote: On Thu, 2009-01-29 at 13:34 -0800, Kaitlin Rupert wrote: Mark McLoughlin wrote: I don't think we want to define a bridge here, but more that an interface is shared - i.e. this is a property of eth2. Note this line. The main concern is that this is the way I'd expect NetworkManager to support it - i.e. that you could configure NetworkManager to share eth0, rather than ask it to create br0 and add eth0 to it. If you just want to create a bridge, you can creati a virtual network. Sorry to chime in so late... the virtual network support only allows the user to define bridges with NAT/routed forwarding. I tripped over this the first time, too; to put words in Mark's mouth, he meant 'bridge' here in the sense of a 'vritual network'. He calls a bridge with one enslaved physical NIC a 'shared network interface' (and doesn't much care for bridges with more NIC's ;) Ah, okay. This makes sense. Thanks for the clarification. After talking to NM people, there doesn't seem to be a concern with NM compat - they are fine with the more explicit representation of a bridge. Would the still behave as a virtual network pool in this case? If multiple guests are tied to the same bridge, it would be useful to represent this as some kind of pool or grouping. Not sure what you mean here ... with all this, you'd still set your guests up the way you do today, with appropriate sources for their network interfaces, e.g. source bridge='br0'/ Would there be the bridge equivalents of virConnectListNetworks()/virConnectListDefinedNetworks(): a way to list the available bridges? After reading through all the mails again, I believe this is what's being proposed, so I think I might have answered my own question. =) -- Kaitlin Rupert IBM Linux Technology Center kait...@linux.vnet.ibm.com -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
