- Devel - libvirt List Archives

[Libvir] RFC: Add a short doc with libvirt coding style guidelines
by Daniel P. Berrange 15 Jan '08

15 Jan '08

There's a few coding style guidelines we've kind of informally agreed upon wrt to libvirt patches, and perhaps more that we ought to make decisions on. For the benefit of people sending patches I reckon its worth writing these down. So I'm attaching a proposed doc 'HACKING' in which we can detail the guidelines. And yes, we don't uniformly comply with these guidelines in all places....all the more reason to list them & fix cases where we don't comply :-) Thoughts.. ? Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

4 8

[Libvir] RFC: PATCH 0/5: asynchronous background jobs
by Daniel P. Berrange 14 Jan '08

14 Jan '08

We currently have a restriction that virConnectPtr objects must only be used by a single thread. At the same time we have a number of operations that can take a very long time. This means that applications are pretty much forced to use multi-threading if they want to continue to respond to user input, and thus are also forced to use multiple virConnectPtr objects. This is sub-optimal because opening multiple virConnectPtr objects may well result in the user being prompted for auth credentials multiple times. It also doesn't provide any way of providing progess information on the long running jobs, nor a way to cancel them. Thus the following series of patches introduces the concept of an 'asynchronous background job', represented by a virJobPtr object in the libvirt public API. A job has an elapsed time, and may optionally include an estimated time of completion. If it is a job with a bounded amount of work, it will also have a percentage completion counter. There is the ability to request cancellation of an in progress job, and fetch any error associated with a completed job. The primary reason I implemented this support, is that the storage APIs will make the current situation untennable. Allocating storage can take a seriously long time (many many minutes if cloning multi-GB files). This absolutely requires progress information and an ability to cancel an operation. There are a number of existing APIs which can benefit from this, hence I decided to work on this separately from the main storage APIs. The APIs which can make use of this are virDomainCreateLinux, virDomainCreate, virNetworkCreate, virNetworkCreateXML, virDomainSave, virDomainRestore, and virDomainDumpCore. For all of these we add a second variant postfixed with 'Job' in the name, returning a virJobPtr object This work isn't finished, but I wanted to put these patches out for comment before going further. In particular I need to figure out how to hook this up to the remote driver and daemon, and deal with a few lifecycle issues. eg what todo if a virConnectPtr object is released while a background job is active. With all 5 patches applied you should be able to run the following test case: virsh --connect test:///default save --verbose test foo No other drivers / commands are hooked up aside from 'save' in the 'test' driver. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 16

[Libvir] [PATCH] examples.xml: Regenerate, now that *.c file names are sorted
by Jim Meyering 14 Jan '08

14 Jan '08

I ran "make distcheck" and noticed these changes in generated-and-version-controlled files: * docs/examples/examples.xml: Regenerate, now that *.c file names are sorted. Signed-off-by: Jim Meyering <meyering(a)redhat.com> --- docs/examples/.cvsignore | 2 +- docs/examples/examples.xml | 50 ++++++++++++++++++++++---------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/docs/examples/.cvsignore b/docs/examples/.cvsignore index aa743cb..5f0e251 100644 --- a/docs/examples/.cvsignore +++ b/docs/examples/.cvsignore @@ -3,5 +3,5 @@ Makefile.in Makefile .deps .libs -suspend info1 +suspend diff --git a/docs/examples/examples.xml b/docs/examples/examples.xml index af7d4b9..5e54567 100644 --- a/docs/examples/examples.xml +++ b/docs/examples/examples.xml @@ -1,4 +1,23 @@ <examples> + <example filename='info1.c'> + <synopsis>Extract informations about Xen domain 0</synopsis> + <purpose>Demonstrate the basic use of the library to connect to the hypervisor and extract domain informations.</purpose> + <usage>info1</usage> + <test>info1</test> + <author>Daniel Veillard</author> + <copy>see Copyright for the status of this software. </copy> + <section>Informations</section> + <includes> + </includes> + <uses> + <function line='43' file='libvirt' name='virDomainGetInfo'/> + <function line='53' file='libvirt' name='virDomainFree'/> + <function line='36' file='libvirt' name='virDomainLookupByID'/> + <function line='55' file='libvirt' name='virConnectClose'/> + <struct line='25' file='libvirt' name='virDomainInfo'/> + <function line='29' file='libvirt' name='virConnectOpenReadOnly'/> + </uses> + </example> <example filename='suspend.c'> <synopsis>Suspend a domain and then resume its execution</synopsis> <purpose>Demonstrate the basic use of the library to suspend and resume a domain. If no id is given on the command line this script will suspend and resume the first domain found which is not Domain 0.</purpose> @@ -21,52 +40,33 @@ <function line='100' file='libvirt' name='virConnectOpenReadOnly'/> </uses> </example> - <example filename='info1.c'> - <synopsis>Extract informations about Xen domain 0</synopsis> - <purpose>Demonstrate the basic use of the library to connect to the hypervisor and extract domain informations.</purpose> - <usage>info1</usage> - <test>info1</test> - <author>Daniel Veillard</author> - <copy>see Copyright for the status of this software. </copy> - <section>Informations</section> - <includes> - </includes> - <uses> - <function line='43' file='libvirt' name='virDomainGetInfo'/> - <function line='53' file='libvirt' name='virDomainFree'/> - <function line='36' file='libvirt' name='virDomainLookupByID'/> - <function line='55' file='libvirt' name='virConnectClose'/> - <struct line='25' file='libvirt' name='virDomainInfo'/> - <function line='29' file='libvirt' name='virConnectOpenReadOnly'/> - </uses> - </example> <symbols> <symbol name='virConnectClose'> - <ref filename='suspend.c'/> <ref filename='info1.c'/> + <ref filename='suspend.c'/> </symbol> <symbol name='virConnectListDomains'> <ref filename='suspend.c'/> </symbol> <symbol name='virConnectOpenReadOnly'> - <ref filename='suspend.c'/> <ref filename='info1.c'/> + <ref filename='suspend.c'/> </symbol> <symbol name='virDomainFree'> - <ref filename='suspend.c'/> <ref filename='info1.c'/> + <ref filename='suspend.c'/> </symbol> <symbol name='virDomainGetInfo'> - <ref filename='suspend.c'/> <ref filename='info1.c'/> + <ref filename='suspend.c'/> </symbol> <symbol name='virDomainInfo'> - <ref filename='suspend.c'/> <ref filename='info1.c'/> + <ref filename='suspend.c'/> </symbol> <symbol name='virDomainLookupByID'> - <ref filename='suspend.c'/> <ref filename='info1.c'/> + <ref filename='suspend.c'/> </symbol> <symbol name='virDomainResume'> <ref filename='suspend.c'/> -- 1.5.4.rc1.11.gd2f82

3 2

[Libvir] [PATCH] Clean up global name space in examples and tests.
by Jim Meyering 14 Jan '08

14 Jan '08

These don't really matter, other than to remove false-positives if/when we ever automate checks for unnecessarily-global symbols. Clean up global name space in examples and tests. * docs/examples/suspend.c: Declare global "conn" to be static. * tests/qemuxml2argvtest.c: Declare global "driver" to be static. * tests/qemuxml2xmltest.c: Likewise. diff --git a/docs/examples/suspend.c b/docs/examples/suspend.c index 80e4da7..9ca6299 100644 --- a/docs/examples/suspend.c +++ b/docs/examples/suspend.c @@ -15,7 +15,7 @@ #include <stdio.h> #include <libvirt/libvirt.h> -virConnectPtr conn = NULL; /* the hypervisor connection */ +static virConnectPtr conn = NULL; /* the hypervisor connection */ /** * checkDomainState: diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d4e39b2..22e2e81 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -16,7 +16,7 @@ static char *progname; static char *abs_top_srcdir; -struct qemud_driver driver; +static struct qemud_driver driver; #define MAX_FILE 4096 diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c42e6fb..2ab1696 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -16,7 +16,7 @@ static char *progname; static char *abs_top_srcdir; -struct qemud_driver driver; +static struct qemud_driver driver; #define MAX_FILE 4096 -- 1.5.4.rc1.11.gd2f82

3 2

[Libvir] [PATCH] Clean up the global name space.
by Jim Meyering 14 Jan '08

14 Jan '08

After mentioning "avoid global variables" as a potential addition to HACKING, I did some quick tests and spotted a few that are easy to eliminate. This is useful even if we don't end up automating the check: Clean up the global name space. * src/qemu_conf.c: Add "static" and "const" attributes to some globals. * src/qemu_conf.h: Update a declaration. * src/qemu_driver.c (qemud_driver): Declare static. diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 15779b7..d03d989 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -231,35 +231,35 @@ void qemudFreeVM(struct qemud_vm *vm) { /* The list of possible machine types for various architectures, as supported by QEMU - taken from 'qemu -M ?' for each arch */ -static const char *arch_info_x86_machines[] = { +static const char *const arch_info_x86_machines[] = { "pc", "isapc", NULL }; -static const char *arch_info_mips_machines[] = { +static const char *const arch_info_mips_machines[] = { "mips", NULL }; -static const char *arch_info_sparc_machines[] = { +static const char *const arch_info_sparc_machines[] = { "sun4m", NULL }; -static const char *arch_info_ppc_machines[] = { +static const char *const arch_info_ppc_machines[] = { "g3bw", "mac99", "prep", NULL }; /* Feature flags for the architecture info */ -struct qemu_feature_flags arch_info_i686_flags [] = { +static const struct qemu_feature_flags const arch_info_i686_flags [] = { { "pae", 1, 1 }, { "acpi", 1, 1 }, { "apic", 1, 0 }, { NULL, -1, -1 } }; -struct qemu_feature_flags arch_info_x86_64_flags [] = { +static const struct qemu_feature_flags const arch_info_x86_64_flags [] = { { "acpi", 1, 1 }, { "apic", 1, 0 }, { NULL, -1, -1 } }; /* The archicture tables for supported QEMU archs */ -struct qemu_arch_info qemudArchs[] = { +const struct qemu_arch_info const qemudArchs[] = { /* i686 must be in position 0 */ { "i686", 32, arch_info_x86_machines, "qemu", arch_info_i686_flags }, /* x86_64 must be in position 1 */ diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 69342cb..e2c7514 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -426,11 +426,11 @@ struct qemu_feature_flags { struct qemu_arch_info { const char *arch; int wordsize; - const char **machines; + const char *const *machines; const char *binary; const struct qemu_feature_flags *fflags; }; -extern struct qemu_arch_info qemudArchs[]; +extern const struct qemu_arch_info const qemudArchs[]; #endif /* WITH_QEMU */ diff --git a/src/qemu_driver.c b/src/qemu_driver.c index c96fb45..dee6ba6 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -107,7 +107,7 @@ static int qemudShutdownNetworkDaemon(virConnectPtr conn, struct qemud_driver *driver, struct qemud_network *network); -struct qemud_driver *qemu_driver = NULL; +static struct qemud_driver *qemu_driver = NULL; static -- 1.5.4.rc1.11.gd2f82

3 2

Re: [Libvir] [patch 7/9] Add support for lokkit
by Richard W.M. Jones 14 Jan '08

14 Jan '08

+AC_ARG_ENABLE(iptables-lokkit, Just a minor nit here: My reading of the autoconf info page is that this should be a --with argument, not an --enable argument. [...] Optional Features: ... --enable-bar include bar Optional Packages: ... --with-foo use foo [...] 14.2 Working With External Software =================================== Some packages require, or can optionally use, other software packages that are already installed. The user can give `configure' command line options to specify which such external software to use. The options have one of these forms: --with-PACKAGE[=ARG] --without-PACKAGE [...] 14.3 Choosing Package Options ============================= If a software package has optional compile-time features, the user can give `configure' command line options to specify whether to compile them. The options have one of these forms: --enable-FEATURE[=ARG] --disable-FEATURE Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

4 4

[Libvir] [patch] qemu/kvm: use_system_powerdown instead of killing the vm hard
by Guido Guenther 14 Jan '08

14 Jan '08

Hi, currently domainShutdown kills qemu/kvm instances hard which is not very filesystem friendly. However recent kvm git acquired system_powerdown to shutdown the system gracefully by simulating an acpi power button press. We can now use this in libvirt: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f792eba..55adb18 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1849,6 +1849,27 @@ static int qemudDomainResume(virDomainPtr dom) { } +static int qemudDomainShutdown(virDomainPtr dom) { + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + struct qemud_vm *vm = qemudFindVMByID(driver, dom->id); + char* info; + + if (!vm) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + "no domain with matching id %d", dom->id); + return -1; + } + + if (qemudMonitorCommand(driver, vm, "system_powerdown", &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "shutdown operation failed"); + return -1; + } + return 0; + +} + + static int qemudDomainDestroy(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; struct qemud_vm *vm = qemudFindVMByID(driver, dom->id); @@ -2855,7 +2876,7 @@ static virDriver qemuDriver = { qemudDomainLookupByName, /* domainLookupByName */ qemudDomainSuspend, /* domainSuspend */ qemudDomainResume, /* domainResume */ - qemudDomainDestroy, /* domainShutdown */ + qemudDomainShutdown, /* domainShutdown */ NULL, /* domainReboot */ qemudDomainDestroy, /* domainDestroy */ qemudDomainGetOSType, /* domainGetOSType */ Please apply, -- Guido

4 14

[Libvir] Error compiling libvirt 0.4.0 on debian etch
by Solli Honorio 14 Jan '08

14 Jan '08

Hi, when I'm trying to compile the libvirt 0.4.0 on Debian etch I'm getting the error below, I have no problem with version 0.3.3. I've look at line 578 of /usr/include/xen/xen.h and this error don't make sense for me. Does any anyone had this problem on debian etch too ? Maybe someone can help-me with this error ? Thanks for attention, Solli Honorio <error> .... In file included from /usr/include/xen/dom0_ops.h:31, from xen_unified.c:29: /usr/include/xen/xen.h:578: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'char' /usr/include/xen/xen.h:579: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'short' /usr/include/xen/xen.h:580: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int' /usr/include/xen/xen.h:581: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'long' make[2]: *** [libvirt_la-xen_unified.lo] Error 1 make[2]: Leaving directory `/usr/src/libvirt-0.4.0/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/usr/src/libvirt-0.4.0' make: *** [all] Error 2 </error>

2 1

[Libvir] [patch 1/1] Reduce the number of times lokkit is run
by Mark McLoughlin 14 Jan '08

14 Jan '08

plain text document attachment (libvirt-iptables-save-rules.patch) With --enable-iptables-lokkit, the rules are saved to disk and lokkit --custom-rules is run each time a single rule is added or removed. This patch moves this code into a new iptablesSaveRules() function so that we can do it for all rules at once. Signed-off-by: Mark McLoughlin <markmc(a)redhat.com> Index: libvirt/src/iptables.c =================================================================== --- libvirt.orig/src/iptables.c 2008-01-04 15:30:31.000000000 +0000 +++ libvirt.orig/src/iptables.c 2008-01-04 15:30:31.000000000 +0000 @@ -282,6 +282,25 @@ writeRules(const char *path, #endif /* ENABLE_IPTABLES_LOKKIT */ static void +iptRulesSave(iptRules *rules) +{ +#ifdef ENABLE_IPTABLES_LOKKIT + int err; + + if ((err = writeRules(rules->path, rules->rules, rules->nrules))) { + qemudLog(QEMUD_WARN, "Failed to saves iptables rules to %s : %s", + rules->path, strerror(err)); + return; + } + + if (rules->nrules > 0) + notifyRulesUpdated(rules->table, rules->path); + else + notifyRulesRemoved(rules->table, rules->path); +#endif /* ENABLE_IPTABLES_LOKKIT */ +} + +static void iptRuleFree(iptRule *rule) { if (rule->rule) @@ -321,20 +340,6 @@ iptRulesAppend(iptRules *rules, rules->nrules++; -#ifdef ENABLE_IPTABLES_LOKKIT - { - int err; - - if ((err = virFileMakePath(rules->dir))) - return err; - - if ((err = writeRules(rules->path, rules->rules, rules->nrules))) - return err; - } - - notifyRulesUpdated(rules->table, rules->path); -#endif /* ENABLE_IPTABLES_LOKKIT */ - return 0; } @@ -359,20 +364,6 @@ iptRulesRemove(iptRules *rules, rules->nrules--; -#ifdef ENABLE_IPTABLES_LOKKIT - { - int err; - - if ((err = writeRules(rules->path, rules->rules, rules->nrules))) - return err; - } - - if (rules->nrules > 0) - notifyRulesUpdated(rules->table, rules->path); - else - notifyRulesRemoved(rules->table, rules->path); -#endif /* ENABLE_IPTABLES_LOKKIT */ - return 0; } @@ -658,6 +649,22 @@ iptablesContextFree(iptablesContext *ctx free(ctx); } +/** + * iptablesSaveRules: + * @ctx: pointer to the IP table context + * + * Saves all the IP table rules associated with a context + * to disk so that if iptables is restarted, the rules + * will automatically be reload. + */ +void +iptablesSaveRules(iptablesContext *ctx) +{ + iptRulesSave(ctx->input_filter); + iptRulesSave(ctx->forward_filter); + iptRulesSave(ctx->nat_postrouting); +} + static void iptRulesReload(iptRules *rules) { Index: libvirt/src/iptables.h =================================================================== --- libvirt.orig/src/iptables.h 2007-12-13 09:05:31.000000000 +0000 +++ libvirt.orig/src/iptables.h 2007-12-13 09:05:31.000000000 +0000 @@ -29,6 +29,7 @@ typedef struct _iptablesContext iptables iptablesContext *iptablesContextNew (void); void iptablesContextFree (iptablesContext *ctx); +void iptablesSaveRules (iptablesContext *ctx); void iptablesReloadRules (iptablesContext *ctx); int iptablesAddTcpInput (iptablesContext *ctx, Index: libvirt/src/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu_driver.c 2007-12-13 09:05:31.000000000 +0000 +++ libvirt.orig/src/qemu_driver.c 2007-12-13 09:05:31.000000000 +0000 @@ -1009,8 +1009,10 @@ qemudAddIptablesRules(virConnectPtr conn /* The remaining rules are only needed for IP forwarding */ - if (!network->def->forward) + if (!network->def->forward) { + iptablesSaveRules(driver->iptables); return 1; + } /* allow forwarding packets from the bridge interface */ if ((err = iptablesAddForwardAllowOut(driver->iptables, @@ -1044,6 +1046,8 @@ qemudAddIptablesRules(virConnectPtr conn goto err10; } + iptablesSaveRules(driver->iptables); + return 1; err10: @@ -1100,6 +1104,7 @@ qemudRemoveIptablesRules(struct qemud_dr iptablesRemoveTcpInput(driver->iptables, network->bridge, 53); iptablesRemoveUdpInput(driver->iptables, network->bridge, 67); iptablesRemoveTcpInput(driver->iptables, network->bridge, 67); + iptablesSaveRules(driver->iptables); } static int --

3 2

[Libvir] PATCH: BZ 427107: fix crash wrt auth callback
by Daniel P. Berrange 14 Jan '08

14 Jan '08

If the application does not supply an authentication callback, and tries to connect to a server with PolicyKit auth turned on it will try to deference a NULL pointer with predictably crashtastic results: https://bugzilla.redhat.com/show_bug.cgi?id=427107 This patch has been tested by bug reporter to fix the issue Index: src/remote_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/remote_internal.c,v retrieving revision 1.49 diff -u -p -r1.49 remote_internal.c --- src/remote_internal.c 17 Dec 2007 17:41:02 -0000 1.49 +++ src/remote_internal.c 31 Dec 2007 20:30:19 -0000 @@ -3347,24 +3347,26 @@ remoteAuthPolkit (virConnectPtr conn, st }; remoteDebug(priv, "Client initialize PolicyKit authentication"); - for (i = 0 ; i < auth->ncredtype ; i++) { - if (auth->credtype[i] == VIR_CRED_EXTERNAL) - allowcb = 1; - } + if (auth && auth->cb) { + /* Check if the neccessary credential type for PolicyKit is supported */ + for (i = 0 ; i < auth->ncredtype ; i++) { + if (auth->credtype[i] == VIR_CRED_EXTERNAL) + allowcb = 1; + } - /* Run the authentication callback */ - if (allowcb) { - if (auth && auth->cb && - (*(auth->cb))(&cred, 1, auth->cbdata) < 0) { - __virRaiseError (in_open ? NULL : conn, NULL, NULL, VIR_FROM_REMOTE, - VIR_ERR_AUTH_FAILED, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "Failed to collect auth credentials"); - return -1; + if (allowcb) { + /* Run the authentication callback */ + if ((*(auth->cb))(&cred, 1, auth->cbdata) < 0) { + __virRaiseError (in_open ? NULL : conn, NULL, NULL, VIR_FROM_REMOTE, + VIR_ERR_AUTH_FAILED, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, + "Failed to collect auth credentials"); + return -1; + } } else { - remoteDebug(priv, "No auth callback provided for PolicyKit"); + remoteDebug(priv, "Client auth callback does not support PolicyKit"); } } else { - remoteDebug(priv, "Client auth callback does not support PolicyKit"); + remoteDebug(priv, "No auth callback provided"); } memset (&ret, 0, sizeof ret); Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 6