Re: [libvirt] [PATCH] virhostdev.h: remove ATTRIBUTE_NONNULL to oldStateDir
On 03/19/2014 04:17 AM, Chunyan Liu wrote: For libxl driver usage, it didn't support hostdev passthrough before, oldStateDir is NULL when calling virHostdevReAttachDomainHostdevs. That is allowed. Remove ATTRIBUTE_NONNULL setting to oldStateDir. s/to/from/ Signed-off-by: Chunyan Liu cy...@suse.com --- src/util/virhostdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK and I've pushed it with the commit message amended. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Add ability to register callback for virCommand dry run
On 03/18/2014 04:00 PM, Daniel P. Berrange wrote: To allow for fault injection of the virCommand dry run, add the ability to register a callback. The callback will be passed the argv, env and stdin buffer and is expected to return the exit status and optionally fill stdout and stderr buffers. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/util/vircommand.c | 52 -- src/util/vircommand.h | 1 + src/util/vircommandpriv.h | 13 ++- tests/virkmodtest.c| 8 +++ tests/virnetdevbandwidthtest.c | 3 ++- 5 files changed, 59 insertions(+), 18 deletions(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 00/13] libxl: add basic support for migration
On 13.03.2014 23:11, Jim Fehlig wrote: V2 of https://www.redhat.com/archives/libvir-list/2014-March/msg00156.html New in this version: not much. I rebased on the hostdev passthrough changes and added a few 'begin phase' checks to fail early if the domain is not migratable. See 13/13 for details. Based on an earlier patch from Chunyan Liu https://www.redhat.com/archives/libvir-list/2013-September/msg00667.html This patch series adds basic migration support to the libxl driver. Follow-up patches can improve pre-migration checks and add support for additional migration flags. Patches 1-12 are almost exclusively code motion, moving functions from the main driver module into the libxl_domain and libxl_conf modules. Patch 13 contains the actual migration impl. Jim Fehlig (13): libxl: move libxlDomainEventQueue to libxl_domain libxl: move libxlDomainManagedSavePath to libxl_domain libxl: move libxlSaveImageOpen to libxl_domain libxl: move libxlVmCleanup{,Job} to libxl_domain libxl: move libxlDomEventsRegister to libxl_domain libxl: move libxlDomainAutoCoreDump to libxl_domain libxl: move libxlDoNodeGetInfo to libxl_conf libxl: move libxlDomainSetVcpuAffinities to libxl_domain libxl: move libxlFreeMem to libxl_domain libxl: move libxlVmStart to libxl_domain libxl: include a pointer to the driver in libxlDomainObjPrivate libxl: move domain event handler to libxl_domain libxl: add migration support po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/libxl/libxl_conf.c | 36 ++ src/libxl/libxl_conf.h | 10 + src/libxl/libxl_domain.c| 705 +++ src/libxl/libxl_domain.h| 51 ++- src/libxl/libxl_driver.c| 988 +++- src/libxl/libxl_migration.c | 598 +++ src/libxl/libxl_migration.h | 78 9 files changed, 1716 insertions(+), 754 deletions(-) create mode 100644 src/libxl/libxl_migration.c create mode 100644 src/libxl/libxl_migration.h ACK series. The first 12 patches are pure code movement (except for 11/13 which is trivial) and therefore can be pushed right now. However, in the last one I have one question, well concern, so I'd suggest to not push it straight away, but give others a few moments to disagree. And push it after that. You know, it's inventing a new protocol which I fear is not extensible (but do we need an extensibility there anyway?). From code POV, it's clean though. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 13/13] libxl: add migration support
On 13.03.2014 23:11, Jim Fehlig wrote: This patch adds initial migration support to the libxl driver, using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration functions. Signed-off-by: Jim Fehlig jfeh...@suse.com --- V2: - Now that the libxl driver supports hostdev passthrough, ensure domain has no hostdevs before migration - Fail early in libxlDomainMigrateBegin3Params if domain is inactive - Change name of local variable in libxlDoMigrateSend from 'live' to 'xl_flags'. po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/libxl/libxl_conf.h | 6 + src/libxl/libxl_domain.h| 1 + src/libxl/libxl_driver.c| 225 + src/libxl/libxl_migration.c | 598 src/libxl/libxl_migration.h | 78 ++ 7 files changed, 911 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3306dc1..6fc5266 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4313,6 +4323,216 @@ cleanup: return ret; } +static char * +libxlDomainMigrateBegin3Params(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ +const char *xmlin = NULL; +const char *dname = NULL; +virDomainObjPtr vm = NULL; + +virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL); +if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) 0) +return NULL; + +if (virTypedParamsGetString(params, nparams, +VIR_MIGRATE_PARAM_DEST_XML, +xmlin) 0 || +virTypedParamsGetString(params, nparams, +VIR_MIGRATE_PARAM_DEST_NAME, +dname) 0) I'd expect @dname to be used somewhere... +return NULL; + +if (!(vm = libxlDomObjFromDomain(domain))) +return NULL; + +if (virDomainMigrateBegin3ParamsEnsureACL(domain-conn, vm-def) 0) { +virObjectUnlock(vm); +return NULL; +} + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +virObjectUnlock(vm); +return NULL; +} + +return libxlDomainMigrationBegin(domain-conn, vm, xmlin); +} + diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c new file mode 100644 index 000..01023db --- /dev/null +++ b/src/libxl/libxl_migration.c @@ -0,0 +1,598 @@ +/* + * libxl_migration.c: methods for handling migration with libxenlight + * + * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Authors: + * Jim Fehlig jfeh...@suse.com + * Chunyan Liu cy...@suse.com + */ + +#include config.h + +#include internal.h +#include virlog.h +#include virerror.h +#include virconf.h +#include datatypes.h +#include virfile.h +#include viralloc.h +#include viruuid.h +#include vircommand.h +#include virstring.h +#include rpc/virnetsocket.h +#include libxl_domain.h +#include libxl_driver.h +#include libxl_conf.h +#include libxl_migration.h + +#define VIR_FROM_THIS VIR_FROM_LIBXL + +typedef struct _libxlMigrateReceiveArgs { +virConnectPtr conn; +virDomainObjPtr vm; + +/* for freeing listen sockets */ +virNetSocketPtr *socks; +size_t nsocks; +} libxlMigrateReceiveArgs; + +static const char libxlMigrateReceiverReady[] = +libvirt libxl migration receiver ready, send binary domain data; +static const char libxlMigrateReceiverFinish[] = +domain received, ready to unpause; So you're using these to sync source and destination on migration. Kudos for using text protocol not a binary blob. I haven't followed v1 closely, so what was resolution on this? I mean, my concern is if this is extensible. + + +static int +libxlCheckMessageBanner(int fd, const char *banner, int banner_sz) +{ +char buf[banner_sz]; +int ret = 0; + +do { +ret = saferead(fd, buf, banner_sz); +} while (ret == -1 errno == EAGAIN); The @fd is in blocking
Re: [libvirt] specifying cirrus ram size
On Wed, Mar 19, 2014 at 12:49:06AM -0500, Serge Hallyn wrote: Quoting Eric Blake (ebl...@redhat.com): On 03/18/2014 03:59 PM, Serge Hallyn wrote: Hi, In order to migrate a VM from an older system with qemu-kvm to a newer one with qemu, the newer qemu needs to be told to use the same vga ram size as qemu-kvm used, 8M. virsh domxml-from-native suggests that the way to specify a 8mb cirrus vga ram size would be to add qemu:commandline qemu:arg value='-global'/ qemu:arg value='cirrus-vga.vgamem_mb=8'/ /qemu:commandline This points out a weakness in our code - qemu:commandline is intentionally unsupported, which means our XML needs an actual parameter for this, rather than forcing you back to qemu:commandline. Or maybe we already have the parameter, in which case the bug is in the tbh I originally expected model type='cirrus' vram='8192' to work, but it seemed to be ignored. Yes, that ought to work but it is possible we only ever plumbed it in for QXL, since historically you couldn't change ram for other drivers. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] domiftune: Reword bandwidth clearing paragraph
s/of value/value of/ Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/virsh.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 3e721e0..0fb8248 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -699,7 +699,7 @@ in a single burst at -Ipeak speed as described in the Network XML documentation at Lhttp://libvirt.org/formatnetwork.html#elementQoS. To clear inbound or outbound settings, use I--inbound or I--outbound -respectfully with average of value zero. +respectfully with average value of zero. If I--live is specified, affect a running guest. If I--config is specified, affect the next boot of a persistent guest. -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSetInterfaceParameters: Allow bandwidth clear out
On 18.03.2014 16:59, Eric Blake wrote: On 03/18/2014 04:01 AM, Michal Privoznik wrote: We allow translation from no_bandwidth to has_bandwidth for a vnic. However, going in the opposite direction is not implemented. It's not limitation of the API rather than internal implementation. Awkward. How about: It's not a limitation of the API, rather a missing part of the internal implementation. The problem is, we correctly detect that user hasn't specified any outbound (say he wants to clear out outbound). However, this gets overwritten by current vnic outbound settings. Then, virNetDevBandwidthSet doesn't change anything. We need to stop overwriting the outbound if users don't want us to. Same applies for inbound. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_driver.c | 15 +++ tools/virsh-domain.c | 4 ++-- tools/virsh.pod| 3 +++ 3 files changed, 16 insertions(+), 6 deletions(-) +To clear inbound or outbound settings, use I--inbound or I--outbound +respectfully with average of value zero. s/of value/value of/ ACK Unfortunately, I've mistakenly pushed this patch already. So I'm proposing a follow up patch which fixes the wording. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domiftune: Reword bandwidth clearing paragraph
On 03/19/2014 11:27 AM, Michal Privoznik wrote: s/of value/value of/ Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/virsh.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK from me as well. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domiftune: Reword bandwidth clearing paragraph
On Wed, Mar 19, 2014 at 11:27:26 +0100, Michal Privoznik wrote: s/of value/value of/ Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/virsh.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 3e721e0..0fb8248 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -699,7 +699,7 @@ in a single burst at -Ipeak speed as described in the Network XML documentation at Lhttp://libvirt.org/formatnetwork.html#elementQoS. To clear inbound or outbound settings, use I--inbound or I--outbound -respectfully with average of value zero. +respectfully with average value of zero. If I--live is specified, affect a running guest. If I--config is specified, affect the next boot of a persistent guest. -- 1.9.0 ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] specifying cirrus ram size
On Mi, 2014-03-19 at 10:22 +, Daniel P. Berrange wrote: On Wed, Mar 19, 2014 at 12:49:06AM -0500, Serge Hallyn wrote: Quoting Eric Blake (ebl...@redhat.com): On 03/18/2014 03:59 PM, Serge Hallyn wrote: Hi, In order to migrate a VM from an older system with qemu-kvm to a newer one with qemu, the newer qemu needs to be told to use the same vga ram size as qemu-kvm used, 8M. virsh domxml-from-native suggests that the way to specify a 8mb cirrus vga ram size would be to add In theory this should not be needed, -M pc-$version and compat properties should take care. In practice there probably is a hickup due to upstream qemu using 8M by default and qemu-kvm using 16M by default. tbh I originally expected model type='cirrus' vram='8192' to work, but it seemed to be ignored. Yes, that ought to work but it is possible we only ever plumbed it in for QXL, since historically you couldn't change ram for other drivers. For cirrus this is pretty pointless. We emulate old, existing hardware here and increasing the memory size buys you nothing as real cirrus hardware isn't able to address more memory. Even the 16M we have today are more than cirrus is able to handle and it being there is more or less a historic accident: qemu-kvm bumped video memory from 8 to 16 MB for both stdvga and cirrus without checking how useful that is. For stdvga I have a rfe bug open ;) cheers, Gerd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Zombie process after open libvirt connection
Hello Michal, I am using libvirt 1.1.3 and perl-Sys-Virt 1.1.3 and perl-5.16 on Fedora 19 x86_64 The zombie process appears after open libvirt connection with qemu-tls, and perl module is binding for libvirt library XS. Here is my running example with zombie process: $ perl test-chldhandle-bug-fixed.pl sleep 15 echo ps axf | grep perl echo [2] 12427 init... pid=12427 while... fork 1 end... pid=12430 receive chld fork 2 end... pid=12431 receive chld 2014-03-19 11:06:38.712+: 12427: info : libvirt version: 1.1.3.1, package: 2.fc19 (Unknown, 2014-03-17-15:02:00, cmar-laptop.lan) 2014-03-19 11:06:38.712+: 12427: warning : virNetTLSContextCheckCertificate:1140 : Certificate check failed Certificate [session] owner does not match the hostname 10.10.4.249 connection open fork 3 end... pid=12432 fork 4 end... pid=12440 12427 pts/2S 0:00 | \_ perl test-chldhandle-bug-fixed.pl 12432 pts/2Z 0:00 | | \_ [perl] defunct 12440 pts/2Z 0:00 | | \_ [perl] defunct 12442 pts/2S+ 0:00 | \_ grep --color=auto perl Regards, -- Carlos Rodrigues Engenheiro de Software Sénior Eurotux Informática, S.A. | www.eurotux.com (t) +351 253 680 300 (m) +351 911 926 110 On Seg, 2014-03-17 at 13:37 +0100, Michal Privoznik wrote: On 17.03.2014 12:13, Carlos Rodrigues wrote: Hell Michal, Thank you for your answer, but this doesn't fix my problem. Run your fixed script and we get the same behavior: $ perl test-chldhandle-bug-fixed.pl init... pid=29713 while... fork 1 end... pid=29716 receive chld fork 2 end... pid=29717 receive chld 2014-03-17 11:10:37.234+: 29713: info : libvirt version: 1.0.5.7, package: 2.fc19 (Fedora Project, 2013-11-17-23:21:57, buildvm-18.phx2.fedoraproject.org) 2014-03-17 11:10:37.234+: 29713: warning : virNetTLSContextCheckCertificate:1099 : Certificate check failed Certificate [session] owner does not match the hostname 10.10.4.249 connection open fork 3 end... pid=29827 fork 4 end... pid=29930 go next... I'm not a perl expert, but I don't think it's a libvirt bug anyhow. Moreover, I don't see any zombies: $ perl test-chldhandle-bug-fixed.pl sleep 5 echo ps axf | grep perl echo [1] 11239 init... pid=11239 while... fork 1 end... pid=11241 receive chld fork 2 end... pid=11242 receive chld connection open 11239 pts/18 S 0:00 | \_ perl test-chldhandle-bug-fixed.pl 11245 pts/18 S+ 0:00 | \_ grep --colour=auto perl fork 3 end... pid=11246 receive chld fork 4 end... pid=11247 receive chld go next... btw: with older version I'm seeing this: 11399 pts/18 S 0:00 | \_ perl test-chldhandle-bug.pl 11401 pts/18 Z 0:00 | | \_ [perl] defunct 11402 pts/18 Z 0:00 | | \_ [perl] defunct 11405 pts/18 S+ 0:00 | \_ grep --colour=auto perl What zombies are you seeing? Perl ones or libvirt or ..,? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: fix framebuffer port setting for HVM domains
On Tue, 2014-03-18 at 15:12 -0600, Jim Fehlig wrote: Daniel P. Berrange wrote: On Tue, Mar 18, 2014 at 12:19:47PM -0600, Jim Fehlig wrote: libxl uses the libxl_vnc_info and libxl_sdl_info fields from the hvm union in libxl_domain_build_info struct when generating QEMU args for VNC or SDL. These fields were left unset by the libxl driver, causing libxl to ignore any user settings. E.g. with graphics type='vnc' port='5950'/ port would be ignored and QEMU would instead be invoked with -vnc 127.0.0.1:0,to=99 Unlike the libxl_domain_config struct, the libxl_domain_build_info contains only a single libxl_vnc_info and libxl_sdl_info, so populate these fields from the first vfb in libxl_domain_config-vfbs. Signed-off-by: Jim Fehlig jfeh...@suse.com Signed-off-by: David Kiarie davidkiar...@gmail.com --- src/libxl/libxl_conf.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aa5d958..cfac847 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1022,6 +1022,20 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, d_config-vkbs = x_vkbs; d_config-num_vfbs = d_config-num_vkbs = nvfbs; +/* + * VNC or SDL info must also be set in libxl_domain_build_info + * for HVM domains. Use the first vfb device. + */ +if (STREQ(def-os.type, hvm)) { +libxl_domain_build_info *b_info = d_config-b_info; +libxl_device_vfb vfb = d_config-vfbs[0]; + +if (libxl_defbool_val(vfb.vnc.enable)) +memcpy(b_info-u.hvm.vnc, vfb.vnc, sizeof(libxl_vnc_info)); +else if (libxl_defbool_val(vfb.sdl.enable)) +memcpy(b_info-u.hvm.sdl, vfb.sdl, sizeof(libxl_sdl_info)); +} + return 0; error: ACK. Thanks, I'll push this shortly. It sure would be nice if there were some kind of API in libxl which took a 'libxl_domain_config*' and returned a char **argv string for the QEMU command line args that it would later invoke. That would let us create a test suite for the XML - libxl_domain_config conversion to spot these kind of problems. Yes, agreed. I don't think we would want to expose such a function to actual users of the library, but I think the infrastructure Ian added in eb5cdc52756e libxl: events: Makefile builds internal unit tests could be used to expose such a function only to the test application which needs it. I meant to CC xen-devel on this patch (done now) since it is rather cumbersome to supply the same info in two places for HVM guests. TBH I thought the library did this copy already, but apparently not. Adding this would need to be done carefully since it is at least in principal possible today to have the emulated VGA vnc/sdl output entirely separate from the first PV FB. The improvement you suggest would also be very welcome. The libxl driver in general needs some in-tree testing love. Although I was a bit worried it would be too short for a GSoC project, I proposed this work as potential project through the openSUSE org https://en.opensuse.org/openSUSE:GSOC_ideas#Implement_a_test_driver_for_the_libvirt_libxl_driver I've only had one semi-serious inquiry thus far, so looks like I'll be doing this myself during free cycles. Regardless of which org it ends up under It might be worth also adding it to http://wiki.xen.org/wiki/GSoc_2014 where it might get more eyes from people thinking about doing Xen stuff. Ian. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 13/16] Use KR style for curly braces in src/network/bridge_driver.c
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/network/bridge_driver.c | 54 ++--- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 59b6c09..20930f3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -380,7 +380,8 @@ networkFindActiveConfigs(virNetworkDriverStatePtr driver) static void -networkAutostartConfigs(virNetworkDriverStatePtr driver) { +networkAutostartConfigs(virNetworkDriverStatePtr driver) +{ size_t i; for (i = 0; i driver-networks.count; i++) { @@ -398,7 +399,8 @@ networkAutostartConfigs(virNetworkDriverStatePtr driver) { #if HAVE_FIREWALLD static DBusHandlerResult firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, - DBusMessage *message, void *user_data) { + DBusMessage *message, void *user_data) +{ virNetworkDriverStatePtr _driverState = user_data; if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, @@ -567,7 +569,8 @@ networkStateAutoStart(void) * files and update its state and the networking */ static int -networkStateReload(void) { +networkStateReload(void) +{ if (!driverState) return 0; @@ -591,7 +594,8 @@ networkStateReload(void) { * Shutdown the QEmu daemon, it will stop all active domains and networks */ static int -networkStateCleanup(void) { +networkStateCleanup(void) +{ if (!driverState) return -1; @@ -2173,7 +2177,8 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver, static virNetworkPtr networkLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { + const unsigned char *uuid) +{ virNetworkDriverStatePtr driver = conn-networkPrivateData; virNetworkObjPtr network; virNetworkPtr ret = NULL; @@ -2199,7 +2204,8 @@ cleanup: } static virNetworkPtr networkLookupByName(virConnectPtr conn, - const char *name) { + const char *name) +{ virNetworkDriverStatePtr driver = conn-networkPrivateData; virNetworkObjPtr network; virNetworkPtr ret = NULL; @@ -2237,12 +2243,14 @@ static virDrvOpenStatus networkOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int networkClose(virConnectPtr conn) { +static int networkClose(virConnectPtr conn) +{ conn-networkPrivateData = NULL; return 0; } -static int networkConnectNumOfNetworks(virConnectPtr conn) { +static int networkConnectNumOfNetworks(virConnectPtr conn) +{ int nactive = 0; size_t i; virNetworkDriverStatePtr driver = conn-networkPrivateData; @@ -2297,7 +2305,8 @@ static int networkConnectListNetworks(virConnectPtr conn, char **const names, in return -1; } -static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) { +static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) +{ int ninactive = 0; size_t i; virNetworkDriverStatePtr driver = conn-networkPrivateData; @@ -2623,7 +2632,8 @@ networkValidate(virNetworkDriverStatePtr driver, return 0; } -static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) { +static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) +{ virNetworkDriverStatePtr driver = conn-networkPrivateData; virNetworkDefPtr def; virNetworkObjPtr network = NULL; @@ -2673,7 +2683,8 @@ cleanup: return ret; } -static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) { +static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) +{ virNetworkDriverStatePtr driver = conn-networkPrivateData; virNetworkDefPtr def = NULL; bool freeDef = true; @@ -2738,7 +2749,8 @@ cleanup: } static int -networkUndefine(virNetworkPtr net) { +networkUndefine(virNetworkPtr net) +{ virNetworkDriverStatePtr driver = net-conn-networkPrivateData; virNetworkObjPtr network; int ret = -1; @@ -2969,7 +2981,8 @@ cleanup: return ret; } -static int networkCreate(virNetworkPtr net) { +static int networkCreate(virNetworkPtr net) +{ virNetworkDriverStatePtr driver = net-conn-networkPrivateData; virNetworkObjPtr network; int ret = -1; @@ -3003,7 +3016,8 @@ cleanup: return ret; } -static int networkDestroy(virNetworkPtr net) { +static int networkDestroy(virNetworkPtr net) +{ virNetworkDriverStatePtr driver = net-conn-networkPrivateData; virNetworkObjPtr network; int ret = -1; @@ -3107,7 +3121,8 @@ cleanup: } static int networkGetAutostart(virNetworkPtr net, - int *autostart) { + int *autostart) +{ virNetworkObjPtr network; int ret = -1; @@ -3127,7 +3142,8 @@ cleanup: } static int networkSetAutostart(virNetworkPtr net, -
[libvirt] [PATCH v2 01/16] Use KR style for curly braces in tests/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- tests/commandhelper.c| 5 +++-- tests/qemuargv2xmltest.c | 3 ++- tests/shunloadhelper.c | 5 +++-- tests/testutils.c| 15 +- tests/testutilslxc.c | 3 ++- tests/testutilsqemu.c| 3 ++- tests/testutilsxen.c | 3 ++- tests/virshtest.c| 54 tests/virusbtest.c | 3 ++- tests/xencapstest.c | 33 +++-- 10 files changed, 84 insertions(+), 43 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 296fbbb..32b8512 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -1,7 +1,7 @@ /* * commandhelper.c: Auxiliary program for commandtest * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,7 +37,8 @@ # define VIR_FROM_THIS VIR_FROM_NONE -static int envsort(const void *a, const void *b) { +static int envsort(const void *a, const void *b) +{ const char *const*astrptr = a; const char *const*bstrptr = b; const char *astr = *astrptr; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 4923c2b..9eb3c49 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -36,7 +36,8 @@ static int blankProblemElements(char *data) static int testCompareXMLToArgvFiles(const char *xml, const char *cmdfile, - bool expect_warning) { + bool expect_warning) +{ char *expectxml = NULL; char *actualxml = NULL; char *cmd = NULL; diff --git a/tests/shunloadhelper.c b/tests/shunloadhelper.c index f2afbe8..128adce 100644 --- a/tests/shunloadhelper.c +++ b/tests/shunloadhelper.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011 Red Hat, Inc. + * Copyright (C) 2011, 2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -38,7 +38,8 @@ static void shunloadError(void *userData ATTRIBUTE_UNUSED, int shunloadStart(void); -int shunloadStart(void) { +int shunloadStart(void) +{ virConnectPtr conn; virSetErrorFunc(NULL, shunloadError); diff --git a/tests/testutils.c b/tests/testutils.c index e21e2f4..179c72a 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -362,7 +362,8 @@ virtTestLoadFile(const char *file, char **buf) #ifndef WIN32 static void virtTestCaptureProgramExecChild(const char *const argv[], - int pipefd) { + int pipefd) +{ size_t i; int open_max; int stdinfd = -1; @@ -629,7 +630,8 @@ virtTestLogContentAndReset(void) static unsigned int -virTestGetFlag(const char *name) { +virTestGetFlag(const char *name) +{ char *flagStr; unsigned int flag; @@ -643,21 +645,24 @@ virTestGetFlag(const char *name) { } unsigned int -virTestGetDebug(void) { +virTestGetDebug(void) +{ if (testDebug == -1) testDebug = virTestGetFlag(VIR_TEST_DEBUG); return testDebug; } unsigned int -virTestGetVerbose(void) { +virTestGetVerbose(void) +{ if (testVerbose == -1) testVerbose = virTestGetFlag(VIR_TEST_VERBOSE); return testVerbose || virTestGetDebug(); } unsigned int -virTestGetExpensive(void) { +virTestGetExpensive(void) +{ if (testExpensive == -1) testExpensive = virTestGetFlag(VIR_TEST_EXPENSIVE); return testExpensive; diff --git a/tests/testutilslxc.c b/tests/testutilslxc.c index 1bc6feb..a1574d3 100644 --- a/tests/testutilslxc.c +++ b/tests/testutilslxc.c @@ -8,7 +8,8 @@ # include domain_conf.h -virCapsPtr testLXCCapsInit(void) { +virCapsPtr testLXCCapsInit(void) +{ virCapsPtr caps; virCapsGuestPtr guest; diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 726501c..f7810f6 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -203,7 +203,8 @@ error: return -1; } -virCapsPtr testQemuCapsInit(void) { +virCapsPtr testQemuCapsInit(void) +{ virCapsPtr caps; virCapsGuestPtr guest; virCapsGuestMachinePtr *machines = NULL; diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c index 1b5ee79..f3216e9 100644 --- a/tests/testutilsxen.c +++ b/tests/testutilsxen.c @@ -7,7 +7,8 @@ #include domain_conf.h -virCapsPtr testXenCapsInit(void) { +virCapsPtr testXenCapsInit(void) +{ struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; diff --git a/tests/virshtest.c b/tests/virshtest.c index 3da5fa4..f7edc02 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -42,7 +42,8 @@ static const char *domname_fc4 = fc4\n\n; static const char *domstate_fc4 = running\n\n; static int testFilterLine(char *buffer, - const char
[libvirt] [PATCH v2 03/16] Use KR style for curly braces in src/util/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/util/vircgroup.c | 9 ++--- src/util/virconf.c | 5 +++-- src/util/virdbus.c | 8 +--- src/util/virerror.c | 3 ++- src/util/vireventpoll.c | 29 +++-- src/util/virhook.c | 11 +++ src/util/virnetdevvportprofile.c | 5 +++-- src/util/virrandom.c | 5 +++-- src/util/virsocketaddr.c | 26 +- src/util/virsysinfo.c| 15 ++- src/util/virthread.c | 5 +++-- src/util/virutil.c | 20 +--- src/util/virutil.h | 12 src/util/viruuid.c | 5 +++-- 14 files changed, 102 insertions(+), 56 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index c5925b1..84847b2 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -419,7 +419,8 @@ virCgroupCopyPlacement(virCgroupPtr group, /* * parent == / + path= = / * parent == /libvirt.service + path == = /libvirt.service - * parent == /libvirt.service + path == foo = /libvirt.service/foo + * parent == /libvirt.service + + * path == foo = /libvirt.service/foo */ if (virAsprintf(group-controllers[i].placement, %s%s%s, @@ -519,8 +520,10 @@ virCgroupDetectPlacement(virCgroupPtr group, /* * selfpath == / + path= - / - * selfpath == /libvirt.service + path == - /libvirt.service - * selfpath == /libvirt.service + path == foo - /libvirt.service/foo + * selfpath == /libvirt.service + + * path == - /libvirt.service + * selfpath == /libvirt.service + + * path == foo - /libvirt.service/foo */ if (typelen == len STREQLEN(typestr, tmp, len) group-controllers[i].mountPoint != NULL diff --git a/src/util/virconf.c b/src/util/virconf.c index b965df7..233ad40 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -1,7 +1,7 @@ /* * virconf.c: parser for a subset of the Python encoded Xen configuration files * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -708,7 +708,8 @@ virConfParseStatement(virConfParserCtxtPtr ctxt) */ static virConfPtr virConfParse(const char *filename, const char *content, int len, - unsigned int flags) { + unsigned int flags) +{ virConfParserCtxt ctxt; ctxt.filename = filename; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index e3236d8..9e29ca9 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1,7 +1,7 @@ /* * virdbus.c: helper for using DBus * - * Copyright (C) 2012-2013 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -217,7 +217,8 @@ static int virDBusTranslateWatchFlags(int dbus_flags) } -static void virDBusWatchFree(void *data) { +static void virDBusWatchFree(void *data) +{ struct virDBusWatch *info = data; VIR_FREE(info); } @@ -296,7 +297,8 @@ static const char virDBusBasicTypes[] = { DBUS_TYPE_SIGNATURE, }; -static bool virDBusIsBasicType(char c) { +static bool virDBusIsBasicType(char c) +{ return !!memchr(virDBusBasicTypes, c, ARRAY_CARDINALITY(virDBusBasicTypes)); } diff --git a/src/util/virerror.c b/src/util/virerror.c index 1d7fa77..9db2452 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -43,7 +43,8 @@ virErrorFunc virErrorHandler = NULL; /* global error handler */ void *virUserData = NULL;/* associated data */ virErrorLogPriorityFunc virErrorLogPriorityFilter = NULL; -static virLogPriority virErrorLevelPriority(virErrorLevel level) { +static virLogPriority virErrorLevelPriority(virErrorLevel level) +{ switch (level) { case VIR_ERR_NONE: return VIR_LOG_INFO; diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c index ea890de..d8a36e9 100644 --- a/src/util/vireventpoll.c +++ b/src/util/vireventpoll.c @@ -1,7 +1,7 @@ /* * vireventpoll.c: Poll based event loop for monitoring file handles * - * Copyright (C) 2007, 2010-2013 Red Hat, Inc. + * Copyright (C) 2007, 2010-2014 Red Hat, Inc. * Copyright (C) 2007 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -106,7 +106,8 @@ static int nextTimer = 1; int virEventPollAddHandle(int fd, int events, virEventHandleCallback cb,
[libvirt] [PATCH v2 02/16] Use KR style for curly braces in src/xen*/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/xen/xen_driver.c | 6 -- src/xen/xen_hypervisor.c | 5 +++-- src/xen/xm_internal.c | 10 +++--- src/xenapi/xenapi_utils.c | 5 +++-- src/xenxs/xen_xm.c| 35 +++ 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 8ceb8b6..2199cb0 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -164,7 +164,8 @@ static virDomainDefPtr xenGetDomainDefForDom(virDomainPtr dom) * until reboot which might be false in future Xen implementations. */ static void -xenNumaInit(virConnectPtr conn) { +xenNumaInit(virConnectPtr conn) +{ virNodeInfo nodeInfo; xenUnifiedPrivatePtr priv; int ret; @@ -1916,7 +1917,8 @@ cleanup: } static int -xenUnifiedDomainUndefine(virDomainPtr dom) { +xenUnifiedDomainUndefine(virDomainPtr dom) +{ return xenUnifiedDomainUndefineFlags(dom, 0); } diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 5ccd5fa..e8eaeeb 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1,7 +1,7 @@ /* * xen_hypervisor.c: direct access to Xen hypervisor level * - * Copyright (C) 2005-2013 Red Hat, Inc. + * Copyright (C) 2005-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -2006,7 +2006,8 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions) } -static int xenHypervisorOnceInit(void) { +static int xenHypervisorOnceInit(void) +{ return xenHypervisorInit(NULL); } diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index fbdd89e..846b79c 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.c: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -94,7 +94,8 @@ static int xenInotifyActive(virConnectPtr conn) /* Release memory associated with a cached config object */ -static void xenXMConfigFree(void *payload, const void *key ATTRIBUTE_UNUSED) { +static void xenXMConfigFree(void *payload, const void *key ATTRIBUTE_UNUSED) +{ xenXMConfCachePtr entry = (xenXMConfCachePtr)payload; virDomainDefFree(entry-def); VIR_FREE(entry-filename); @@ -1117,7 +1118,10 @@ struct xenXMListIteratorContext { }; static void -xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { +xenXMListIterator(void *payload ATTRIBUTE_UNUSED, + const void *name, + void *data) +{ struct xenXMListIteratorContext *ctx = data; virDomainDefPtr def = NULL; diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c index 610e0f0..5a5025a 100644 --- a/src/xenapi/xenapi_utils.c +++ b/src/xenapi/xenapi_utils.c @@ -1,6 +1,6 @@ /* * xenapi_utils.c: Xen API driver -- utils parts. - * Copyright (C) 2011-2013 Red Hat, Inc. + * Copyright (C) 2011-2014 Red Hat, Inc. * Copyright (C) 2009, 2010 Citrix Ltd. * * This library is free software; you can redistribute it and/or @@ -181,7 +181,8 @@ createXenAPIBootOrderString(int nboot, int *bootDevs) /* convert boot order string to libvirt boot order enum */ enum virDomainBootOrder -map2LibvirtBootOrder(char c) { +map2LibvirtBootOrder(char c) +{ switch (c) { case 'a': return VIR_DOMAIN_BOOT_FLOPPY; diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index a70c5e3..fce074a 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1,7 +1,7 @@ /* * xen_xm.c: Xen XM parsing functions * - * Copyright (C) 2006-2007, 2009-2010, 2012-2013 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2010, 2012-2014 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2006 Daniel P. Berrange * @@ -44,7 +44,8 @@ static int xenXMConfigGetBool(virConfPtr conf, const char *name, int *value, - int def) { + int def) +{ virConfValuePtr val; *value = 0; @@ -70,7 +71,8 @@ static int xenXMConfigGetBool(virConfPtr conf, static int xenXMConfigGetULong(virConfPtr conf, const char *name, unsigned long *value, - unsigned long def) { + unsigned long def) +{ virConfValuePtr val; *value = 0; @@ -102,7 +104,8 @@ static int xenXMConfigGetULong(virConfPtr conf, static int xenXMConfigGetULongLong(virConfPtr conf, const char *name, unsigned long long *value, - unsigned long long def) { +
[libvirt] [PATCH v2 04/16] Use KR style for curly braces in src/rpc/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/rpc/virnetserver.c | 8 +--- src/rpc/virnetserverclient.c | 5 +++-- src/rpc/virnettlscontext.c | 5 +++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index cea2b23..14fd102 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1,7 +1,7 @@ /* * virnetserver.c: generic network RPC server * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2012, 2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -850,7 +850,8 @@ static void virNetServerSignalEvent(int watch, int fd ATTRIBUTE_UNUSED, int events ATTRIBUTE_UNUSED, -void *opaque) { +void *opaque) +{ virNetServerPtr srv = opaque; siginfo_t siginfo; size_t i; @@ -1021,7 +1022,8 @@ int virNetServerSetTLSContext(virNetServerPtr srv, static void virNetServerAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED, - void *opaque) { + void *opaque) +{ virNetServerPtr srv = opaque; virObjectLock(srv); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 495b0b3..1251b2d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1,7 +1,7 @@ /* * virnetserverclient.c: generic network RPC server client * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -141,7 +141,8 @@ static int virNetServerClientSendMessageLocked(virNetServerClientPtr client, * @client: a locked client object */ static int -virNetServerClientCalculateHandleMode(virNetServerClientPtr client) { +virNetServerClientCalculateHandleMode(virNetServerClientPtr client) +{ int mode = 0; diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 7bf2a2e..4eaf469 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1,7 +1,7 @@ /* * virnettlscontext.c: TLS encryption/x509 handling * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -125,7 +125,8 @@ virNetTLSContextCheckCertFile(const char *type, const char *file, bool allowMiss static void virNetTLSLog(int level ATTRIBUTE_UNUSED, - const char *str ATTRIBUTE_UNUSED) { + const char *str ATTRIBUTE_UNUSED) +{ VIR_DEBUG(%d %s, level, str); } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 11/16] Use KR style for curly braces in src/uml/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/uml/uml_conf.c | 5 ++-- src/uml/uml_driver.c | 78 ++-- 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 3567b03..53a880f 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -1,7 +1,7 @@ /* * uml_conf.c: UML driver configuration * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -52,7 +52,8 @@ VIR_LOG_INIT(uml.uml_conf); -virCapsPtr umlCapsInit(void) { +virCapsPtr umlCapsInit(void) +{ virCapsPtr caps; virCapsGuestPtr guest; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 28e65f4..f5eb05f 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -209,7 +209,8 @@ umlAutostartDomain(virDomainObjPtr vm, } static void -umlAutostartConfigs(struct uml_driver *driver) { +umlAutostartConfigs(struct uml_driver *driver) +{ /* XXX: Figure out a better way todo this. The domain * startup code needs a connection handle in order * to lookup the bridge associated with a virtual @@ -622,7 +623,8 @@ static void umlNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) * files and update its state and the networking */ static int -umlStateReload(void) { +umlStateReload(void) +{ if (!uml_driver) return 0; @@ -660,7 +662,8 @@ umlShutdownOneVM(virDomainObjPtr dom, void *opaque) * Shutdown the Uml daemon, it will stop all active domains and networks */ static int -umlStateCleanup(void) { +umlStateCleanup(void) +{ if (!uml_driver) return -1; @@ -859,7 +862,8 @@ static int umlMonitorAddress(const struct uml_driver *driver, } static int umlOpenMonitor(struct uml_driver *driver, - virDomainObjPtr vm) { + virDomainObjPtr vm) +{ struct sockaddr_un addr; struct stat sb; int retries = 0; @@ -1007,7 +1011,8 @@ error: } -static void umlCleanupTapDevices(virDomainObjPtr vm) { +static void umlCleanupTapDevices(virDomainObjPtr vm) +{ size_t i; for (i = 0; i vm-def-nnets; i++) { @@ -1024,7 +1029,8 @@ static void umlCleanupTapDevices(virDomainObjPtr vm) { static int umlStartVMDaemon(virConnectPtr conn, struct uml_driver *driver, virDomainObjPtr vm, -bool autoDestroy) { +bool autoDestroy) +{ int ret = -1; char *logfile; int logfd = -1; @@ -1245,7 +1251,8 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int umlConnectClose(virConnectPtr conn) { +static int umlConnectClose(virConnectPtr conn) +{ struct uml_driver *driver = conn-privateData; umlDriverLock(driver); @@ -1343,7 +1350,8 @@ static int umlGetProcessInfo(unsigned long long *cpuTime, pid_t pid) static virDomainPtr umlDomainLookupByID(virConnectPtr conn, - int id) { + int id) +{ struct uml_driver *driver = (struct uml_driver *)conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1370,7 +1378,8 @@ cleanup: } static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn, -const unsigned char *uuid) { +const unsigned char *uuid) +{ struct uml_driver *driver = (struct uml_driver *)conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1397,7 +1406,8 @@ cleanup: } static virDomainPtr umlDomainLookupByName(virConnectPtr conn, -const char *name) { +const char *name) +{ struct uml_driver *driver = (struct uml_driver *)conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1500,7 +1510,8 @@ cleanup: return ret; } -static int umlConnectGetVersion(virConnectPtr conn, unsigned long *version) { +static int umlConnectGetVersion(virConnectPtr conn, unsigned long *version) +{ struct uml_driver *driver = conn-privateData; struct utsname ut; int ret = -1; @@ -1538,7 +1549,8 @@ static char *umlConnectGetHostname(virConnectPtr conn) } -static int umlConnectListDomains(virConnectPtr conn, int *ids, int nids) { +static int umlConnectListDomains(virConnectPtr conn, int *ids, int nids) +{ struct uml_driver *driver = conn-privateData; int n; @@ -1552,7 +1564,8 @@ static int umlConnectListDomains(virConnectPtr conn, int *ids, int nids) { return n; } -static int umlConnectNumOfDomains(virConnectPtr conn) { +static int umlConnectNumOfDomains(virConnectPtr conn) +{ struct uml_driver *driver = conn-privateData;
[libvirt] [PATCH v2 12/16] Use KR style for curly braces in src/lxc/lxc_driver.c
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/lxc/lxc_driver.c | 30 -- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1ae04c5..3104bf9 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -384,7 +384,8 @@ cleanup: return ret; } -static int lxcConnectListDomains(virConnectPtr conn, int *ids, int nids) { +static int lxcConnectListDomains(virConnectPtr conn, int *ids, int nids) +{ virLXCDriverPtr driver = conn-privateData; int n; @@ -397,7 +398,8 @@ static int lxcConnectListDomains(virConnectPtr conn, int *ids, int nids) { return n; } -static int lxcConnectNumOfDomains(virConnectPtr conn) { +static int lxcConnectNumOfDomains(virConnectPtr conn) +{ virLXCDriverPtr driver = conn-privateData; int n; @@ -411,7 +413,8 @@ static int lxcConnectNumOfDomains(virConnectPtr conn) { } static int lxcConnectListDefinedDomains(virConnectPtr conn, -char **const names, int nnames) { +char **const names, int nnames) +{ virLXCDriverPtr driver = conn-privateData; int n; @@ -425,7 +428,8 @@ static int lxcConnectListDefinedDomains(virConnectPtr conn, } -static int lxcConnectNumOfDefinedDomains(virConnectPtr conn) { +static int lxcConnectNumOfDefinedDomains(virConnectPtr conn) +{ virLXCDriverPtr driver = conn-privateData; int n; @@ -677,7 +681,8 @@ cleanup: return ret; } -static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { +static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) +{ virDomainObjPtr vm; int ret = -1; @@ -702,7 +707,8 @@ cleanup: return ret; } -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) { +static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) +{ virDomainObjPtr vm; int ret = -1; virLXCDomainObjPrivatePtr priv; @@ -1130,7 +1136,8 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, const char *xml, unsigned int nfiles, int *files, -unsigned int flags) { +unsigned int flags) +{ virLXCDriverPtr driver = conn-privateData; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; @@ -1206,7 +1213,8 @@ cleanup: static virDomainPtr lxcDomainCreateXML(virConnectPtr conn, const char *xml, - unsigned int flags) { + unsigned int flags) +{ return lxcDomainCreateXMLWithFiles(conn, xml, 0, NULL, flags); } @@ -1639,7 +1647,8 @@ static void lxcNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) * files and perform autostart */ static int -lxcStateReload(void) { +lxcStateReload(void) +{ virLXCDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; @@ -3102,7 +3111,8 @@ lxcDomainInterfaceStats(virDomainPtr dom, #endif static int lxcDomainGetAutostart(virDomainPtr dom, - int *autostart) { + int *autostart) +{ virDomainObjPtr vm; int ret = -1; -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 16/16] Require KR styled curly braces around function bodies
Although not explicitly requested, we are using KR (or Kernel) indentation for curly braces around functions in HACKING file and most of the code. Using grep -P, this patch add the syntax-check rule for it (while skipping all the false positives with foreach constructs). Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: Unfortunately, the regexp is meant to catch as much as possible and thus one line functions must occupy two lines, e.g.: static inline int getuid(void) { return 0; } is not valid. This can be changed by appending $$ to the end of the regexp which I didn't want to do since it might not catch something else I haven't thought of. Anyway, feel free to request such change and I'll push it changed without any modifications to such one-liners. cfg.mk | 7 +++ 1 file changed, 7 insertions(+) diff --git a/cfg.mk b/cfg.mk index 7a65d1e..559f719 100644 --- a/cfg.mk +++ b/cfg.mk @@ -898,6 +898,13 @@ sc_prohibit_virConnectOpen_in_virsh: halt='Use vshConnect() in virsh instead of virConnectOpen*'\ $(_sc_search_regexp) +sc_curly_braces_style: + @files=$$($(VC_LIST_EXCEPT) | grep '\.[ch]$$');\ + $(GREP) -nHP \ +'^\s*(?!([a-zA-Z_]*for_?each[a-zA-Z_]*) ?\()([_a-zA-Z0-9]+( [_a-zA-Z0-9]+)* ?\()?(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+|void)\) ?\{' \ + $$files { echo '$(ME): Non-KR style used for curly'\ + 'braces around function body, see' \ + 'HACKING' 12; exit 1; } || : # We don't use this feature of maint.mk. prev_version_file = /dev/null -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/16] Use KR style for curly braces in src/qemu/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_agent.c| 5 ++- src/qemu/qemu_command.c | 6 ++- src/qemu/qemu_driver.c | 94 +--- src/qemu/qemu_migration.c| 3 +- src/qemu/qemu_monitor.c | 3 +- src/qemu/qemu_monitor_json.c | 3 +- src/qemu/qemu_monitor_text.c | 20 +++--- 7 files changed, 90 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 83f077f..9f02e52 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1,7 +1,7 @@ /* * qemu_agent.c: interaction with QEMU guest agent * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -574,7 +574,8 @@ static void qemuAgentUpdateWatch(qemuAgentPtr mon) static void -qemuAgentIO(int watch, int fd, int events, void *opaque) { +qemuAgentIO(int watch, int fd, int events, void *opaque) +{ qemuAgentPtr mon = opaque; bool error = false; bool eof = false; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5ca3f74..2311d89 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1871,7 +1871,8 @@ cleanup: } static bool -qemuDomainSupportsPCI(virDomainDefPtr def) { +qemuDomainSupportsPCI(virDomainDefPtr def) +{ if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) return true; @@ -11197,7 +11198,8 @@ error: static void -qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str) { +qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str) +{ int n, b = 0; for (n = 0; str[n] b VIR_DOMAIN_BOOT_LAST; n++) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2707bec..20239f1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -172,9 +172,11 @@ virQEMUDriverPtr qemu_driver = NULL; static void -qemuVMDriverLock(void) {} +qemuVMDriverLock(void) +{} static void -qemuVMDriverUnlock(void) {} +qemuVMDriverUnlock(void) +{} static int qemuVMFilterRebuild(virDomainObjListIterator iter, void *data) @@ -879,7 +881,8 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque) * files and update its state and the networking */ static int -qemuStateReload(void) { +qemuStateReload(void) +{ virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; @@ -910,7 +913,8 @@ cleanup: * */ static int -qemuStateStop(void) { +qemuStateStop(void) +{ int ret = -1; virConnectPtr conn; int numDomains = 0; @@ -967,7 +971,8 @@ qemuStateStop(void) { * Shutdown the QEmu daemon, it will stop all active domains and networks */ static int -qemuStateCleanup(void) { +qemuStateCleanup(void) +{ if (!qemu_driver) return -1; @@ -1145,7 +1150,8 @@ static int qemuConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) static int -kvmGetMaxVCPUs(void) { +kvmGetMaxVCPUs(void) +{ int fd; int ret; @@ -1201,7 +1207,9 @@ qemuConnectGetSysinfo(virConnectPtr conn, unsigned int flags) return virBufferContentAndReset(buf); } -static int qemuConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) { +static int +qemuConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) +{ if (virConnectGetMaxVcpusEnsureACL(conn) 0) return -1; @@ -1321,7 +1329,8 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, - int id) { + int id) +{ virQEMUDriverPtr driver = conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1347,7 +1356,8 @@ cleanup: } static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { + const unsigned char *uuid) +{ virQEMUDriverPtr driver = conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1375,7 +1385,8 @@ cleanup: } static virDomainPtr qemuDomainLookupByName(virConnectPtr conn, - const char *name) { + const char *name) +{ virQEMUDriverPtr driver = conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -1458,7 +1469,8 @@ cleanup: return ret; } -static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) { +static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) +{ virQEMUDriverPtr driver = conn-privateData; int ret = -1; unsigned int qemuVersion = 0; @@ -1493,7 +1505,8 @@ static char *qemuConnectGetHostname(virConnectPtr conn) } -static int qemuConnectListDomains(virConnectPtr conn, int *ids, int
[libvirt] [PATCH v2 08/16] Use KR style for curly braces in src/openvz/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/openvz/openvz_conf.c | 15 ++- src/openvz/openvz_driver.c | 45 ++--- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 11f048b..20c9a6f 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -205,7 +205,8 @@ no_memory: int openvzReadNetworkConf(virDomainDefPtr def, - int veid) { + int veid) +{ int ret; virDomainNetDefPtr net = NULL; char *temp = NULL; @@ -378,7 +379,8 @@ openvz_replace(const char* str, static int openvzReadFSConf(virDomainDefPtr def, - int veid) { + int veid) +{ int ret; virDomainFSDefPtr fs = NULL; char *veid_str = NULL; @@ -545,7 +547,8 @@ openvzFreeDriver(struct openvz_driver *driver) -int openvzLoadDomains(struct openvz_driver *driver) { +int openvzLoadDomains(struct openvz_driver *driver) +{ int veid, ret; char *status; char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1063,7 +1066,8 @@ cleanup: } static int -openvzSetUUID(int vpsid){ +openvzSetUUID(int vpsid) +{ unsigned char uuid[VIR_UUID_BUFLEN]; if (virUUIDGenerate(uuid)) { @@ -1128,7 +1132,8 @@ static int openvzAssignUUIDs(void) * */ -int openvzGetVEID(const char *name) { +int openvzGetVEID(const char *name) +{ virCommandPtr cmd; char *outbuf; char *temp; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 2a28ed2..526ddbd 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -299,7 +299,8 @@ error: static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, - int id) { + int id) +{ struct openvz_driver *driver = conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -323,7 +324,8 @@ cleanup: return dom; } -static int openvzConnectGetVersion(virConnectPtr conn, unsigned long *version) { +static int openvzConnectGetVersion(virConnectPtr conn, unsigned long *version) +{ struct openvz_driver *driver = conn-privateData; openvzDriverLock(driver); *version = driver-version; @@ -363,7 +365,8 @@ cleanup: static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { + const unsigned char *uuid) +{ struct openvz_driver *driver = conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -388,7 +391,8 @@ cleanup: } static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, - const char *name) { + const char *name) +{ struct openvz_driver *driver = conn-privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; @@ -413,7 +417,8 @@ cleanup: } static int openvzDomainGetInfo(virDomainPtr dom, - virDomainInfoPtr info) { + virDomainInfoPtr info) +{ struct openvz_driver *driver = dom-conn-privateData; virDomainObjPtr vm; int state; @@ -579,7 +584,8 @@ static void openvzSetProgramSentinal(const char **prog, const char *key) } } -static int openvzDomainSuspend(virDomainPtr dom) { +static int openvzDomainSuspend(virDomainPtr dom) +{ struct openvz_driver *driver = dom-conn-privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINEL, --suspend, NULL}; @@ -617,7 +623,8 @@ cleanup: return ret; } -static int openvzDomainResume(virDomainPtr dom) { +static int openvzDomainResume(virDomainPtr dom) +{ struct openvz_driver *driver = dom-conn-privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINEL, --resume, NULL}; @@ -657,7 +664,8 @@ cleanup: static int openvzDomainShutdownFlags(virDomainPtr dom, - unsigned int flags) { + unsigned int flags) +{ struct openvz_driver *driver = dom-conn-privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, --quiet, stop, PROGRAM_SENTINEL, NULL}; @@ -1476,7 +1484,8 @@ cleanup: return VIR_DRV_OPEN_ERROR; }; -static int openvzConnectClose(virConnectPtr conn) { +static int openvzConnectClose(virConnectPtr conn) +{ struct openvz_driver *driver = conn-privateData; openvzFreeDriver(driver); @@ -1489,12 +1498,14 @@ static const char *openvzConnectGetType(virConnectPtr conn ATTRIBUTE_UNUSED) { return OpenVZ; } -static int openvzConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) { +static int openvzConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ /* Encryption is not relevant / applicable to way we talk to openvz */ return 0; }
[libvirt] [PATCH v2 05/16] Use KR style for curly braces in src/conf/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/conf/domain_conf.c | 9 +--- src/conf/domain_nwfilter.c | 13 +++ src/conf/interface_conf.c | 54 ++ src/conf/nwfilter_conf.c | 30 +- src/conf/nwfilter_params.c | 3 ++- 5 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89aa52c..081ec8d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6193,7 +6193,8 @@ virDomainNetGenerateMAC(virDomainXMLOptionPtr xmlopt, static virDomainFSDefPtr virDomainFSDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, - unsigned int flags) { + unsigned int flags) +{ virDomainFSDefPtr def; xmlNodePtr cur, save_node = ctxt-node; char *type = NULL; @@ -7013,7 +7014,8 @@ error: } static int -virDomainChrDefaultTargetType(int devtype) { +virDomainChrDefaultTargetType(int devtype) +{ switch ((enum virDomainChrDeviceType) devtype) { case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: virReportError(VIR_ERR_XML_ERROR, @@ -7419,7 +7421,8 @@ error: * default port. */ virDomainChrDefPtr -virDomainChrDefNew(void) { +virDomainChrDefNew(void) +{ virDomainChrDefPtr def = NULL; if (VIR_ALLOC(def) 0) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index 688a200..6330f67 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -1,6 +1,7 @@ /* * domain_nwfilter.c: * + * Copyright (C) 2014 Red Hat, Inc. * Copyright (C) 2010 IBM Corporation * * This library is free software; you can redistribute it and/or @@ -31,14 +32,16 @@ static virDomainConfNWFilterDriverPtr nwfilterDriver; void -virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) { +virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) +{ nwfilterDriver = driver; } int virDomainConfNWFilterInstantiate(virConnectPtr conn, const unsigned char *vmuuid, - virDomainNetDefPtr net) { + virDomainNetDefPtr net) +{ if (nwfilterDriver != NULL) return nwfilterDriver-instantiateFilter(conn, vmuuid, net); /* driver module not available -- don't indicate failure */ @@ -46,13 +49,15 @@ virDomainConfNWFilterInstantiate(virConnectPtr conn, } void -virDomainConfNWFilterTeardown(virDomainNetDefPtr net) { +virDomainConfNWFilterTeardown(virDomainNetDefPtr net) +{ if (nwfilterDriver != NULL) nwfilterDriver-teardownFilter(net); } void -virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) { +virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) +{ size_t i; if (nwfilterDriver != NULL) { diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 9f065bf..83cc0a9 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -44,7 +44,8 @@ static int virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def); static -void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { +void virInterfaceIpDefFree(virInterfaceIpDefPtr def) +{ if (def == NULL) return; VIR_FREE(def-address); @@ -52,7 +53,8 @@ void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { } static -void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def) { +void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def) +{ size_t i; if (def == NULL) @@ -112,7 +114,8 @@ void virInterfaceDefFree(virInterfaceDefPtr def) static int virInterfaceDefParseName(virInterfaceDefPtr def, - xmlXPathContextPtr ctxt) { + xmlXPathContextPtr ctxt) +{ char *tmp; tmp = virXPathString(string(./@name), ctxt); @@ -127,7 +130,8 @@ virInterfaceDefParseName(virInterfaceDefPtr def, static int virInterfaceDefParseMtu(virInterfaceDefPtr def, -xmlXPathContextPtr ctxt) { +xmlXPathContextPtr ctxt) +{ unsigned long mtu; int ret; @@ -144,7 +148,8 @@ virInterfaceDefParseMtu(virInterfaceDefPtr def, static int virInterfaceDefParseStartMode(virInterfaceDefPtr def, - xmlXPathContextPtr ctxt) { + xmlXPathContextPtr ctxt) +{ char *tmp; tmp = virXPathString(string(./start/@mode), ctxt); @@ -167,7 +172,8 @@ virInterfaceDefParseStartMode(virInterfaceDefPtr def, } static int -virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt) { +virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt) +{ char *tmp; int ret = 0; @@ -198,7 +204,8 @@ virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt) { } static int -virInterfaceDefParseBondMiiCarrier(xmlXPathContextPtr ctxt) { +virInterfaceDefParseBondMiiCarrier(xmlXPathContextPtr ctxt) +{ char *tmp; int ret = 0; @@ -219,7 +226,8 @@
[libvirt] [PATCH v2 14/16] Use KR style for curly braces in src/vbox/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/vbox/vbox_driver.c | 5 +- src/vbox/vbox_tmpl.c | 188 + 2 files changed, 132 insertions(+), 61 deletions(-) diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c index e26b10a..7d004b2 100644 --- a/src/vbox/vbox_driver.c +++ b/src/vbox/vbox_driver.c @@ -3,7 +3,7 @@ */ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2008-2009 Sun Microsystems, Inc. * * This file is part of a free software library; you can redistribute @@ -79,7 +79,8 @@ static virDriver vboxDriverDummy; #define VIR_FROM_THIS VIR_FROM_VBOX -int vboxRegister(void) { +int vboxRegister(void) +{ virDriverPtrdriver; virNetworkDriverPtr networkDriver; virStorageDriverPtr storageDriver; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 2aeddd0..56b3ac6 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -284,17 +284,20 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml); static int vboxDomainCreate(virDomainPtr dom); static int vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags); -static void vboxDriverLock(vboxGlobalData *data) { +static void vboxDriverLock(vboxGlobalData *data) +{ virMutexLock(data-lock); } -static void vboxDriverUnlock(vboxGlobalData *data) { +static void vboxDriverUnlock(vboxGlobalData *data) +{ virMutexUnlock(data-lock); } #if VBOX_API_VERSION == 2002000 -static void nsIDtoChar(unsigned char *uuid, const nsID *iid) { +static void nsIDtoChar(unsigned char *uuid, const nsID *iid) +{ char uuidstrsrc[VIR_UUID_STRING_BUFLEN]; char uuidstrdst[VIR_UUID_STRING_BUFLEN]; unsigned char uuidinterim[VIR_UUID_BUFLEN]; @@ -334,7 +337,8 @@ static void nsIDtoChar(unsigned char *uuid, const nsID *iid) { ignore_value(virUUIDParse(uuidstrdst, uuid)); } -static void nsIDFromChar(nsID *iid, const unsigned char *uuid) { +static void nsIDFromChar(nsID *iid, const unsigned char *uuid) +{ char uuidstrsrc[VIR_UUID_STRING_BUFLEN]; char uuidstrdst[VIR_UUID_STRING_BUFLEN]; unsigned char uuidinterim[VIR_UUID_BUFLEN]; @@ -621,7 +625,8 @@ static char *vboxGenerateMediumName(PRUint32 storageBus, PRInt32 devicePort, PRInt32 deviceSlot, PRUint32 *aMaxPortPerInst, -PRUint32 *aMaxSlotPerPort) { +PRUint32 *aMaxSlotPerPort) +{ const char *prefix = NULL; char *name = NULL; int total = 0; @@ -734,7 +739,8 @@ static bool vboxGetDeviceDetails(const char *deviceName, static bool vboxGetMaxPortSlotValues(IVirtualBox *vbox, PRUint32 *maxPortPerInst, - PRUint32 *maxSlotPerPort) { + PRUint32 *maxSlotPerPort) +{ ISystemProperties *sysProps = NULL; if (!vbox) @@ -779,7 +785,8 @@ static bool vboxGetMaxPortSlotValues(IVirtualBox *vbox, /** * Converts Utf-16 string to int */ -static int PRUnicharToInt(PRUnichar *strUtf16) { +static int PRUnicharToInt(PRUnichar *strUtf16) +{ char *strUtf8 = NULL; int ret = 0; @@ -957,7 +964,8 @@ cleanup: return -1; } -static int vboxExtractVersion(vboxGlobalData *data) { +static int vboxExtractVersion(vboxGlobalData *data) +{ int ret = -1; PRUnichar *versionUtf16 = NULL; nsresult rc; @@ -985,7 +993,8 @@ static int vboxExtractVersion(vboxGlobalData *data) { return ret; } -static void vboxUninitialize(vboxGlobalData *data) { +static void vboxUninitialize(vboxGlobalData *data) +{ if (!data) return; @@ -1078,7 +1087,8 @@ static virDrvOpenStatus vboxConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int vboxConnectClose(virConnectPtr conn) { +static int vboxConnectClose(virConnectPtr conn) +{ vboxGlobalData *data = conn-privateData; VIR_DEBUG(%s: in vboxClose, conn-driver-name); @@ -1088,7 +1098,8 @@ static int vboxConnectClose(virConnectPtr conn) { return 0; } -static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version) { +static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version) +{ vboxGlobalData *data = conn-privateData; VIR_DEBUG(%s: in vboxGetVersion, conn-driver-name); @@ -1106,12 +1117,14 @@ static char *vboxConnectGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) } -static int vboxConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) { +static int vboxConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ /* Driver is using local, non-network based transport */ return 1; } -static int vboxConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) { +static int vboxConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ /*
[libvirt] [PATCH v2 09/16] Use KR style for curly braces in src/nwfilter/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/nwfilter/nwfilter_driver.c| 32 +++- src/nwfilter/nwfilter_ebiptables_driver.c | 3 ++- src/nwfilter/nwfilter_learnipaddr.c | 41 --- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 0876901..4fab1f2 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -2,7 +2,7 @@ * nwfilter_driver.c: core driver for network filter APIs *(based on storage_driver.c) * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2011, 2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * Copyright (C) 2010 IBM Corporation * Copyright (C) 2010 Stefan Berger @@ -325,7 +325,8 @@ virNWFilterDriverIsWatchingFirewallD(void) * Shutdown the nwfilter driver, it will stop all active nwfilters */ static int -nwfilterStateCleanup(void) { +nwfilterStateCleanup(void) +{ if (!driverState) return -1; @@ -356,7 +357,8 @@ nwfilterStateCleanup(void) { static virNWFilterPtr nwfilterLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { + const unsigned char *uuid) +{ virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; virNWFilterObjPtr nwfilter; virNWFilterPtr ret = NULL; @@ -385,7 +387,8 @@ cleanup: static virNWFilterPtr nwfilterLookupByName(virConnectPtr conn, - const char *name) { + const char *name) +{ virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; virNWFilterObjPtr nwfilter; virNWFilterPtr ret = NULL; @@ -428,14 +431,16 @@ nwfilterOpen(virConnectPtr conn, static int -nwfilterClose(virConnectPtr conn) { +nwfilterClose(virConnectPtr conn) +{ conn-nwfilterPrivateData = NULL; return 0; } static int -nwfilterConnectNumOfNWFilters(virConnectPtr conn) { +nwfilterConnectNumOfNWFilters(virConnectPtr conn) +{ virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; size_t i; int n; @@ -459,7 +464,8 @@ nwfilterConnectNumOfNWFilters(virConnectPtr conn) { static int nwfilterConnectListNWFilters(virConnectPtr conn, char **const names, - int nnames) { + int nnames) +{ virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; int got = 0; size_t i; @@ -495,7 +501,8 @@ nwfilterConnectListNWFilters(virConnectPtr conn, static int nwfilterConnectListAllNWFilters(virConnectPtr conn, virNWFilterPtr **filters, -unsigned int flags) { +unsigned int flags) +{ virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; virNWFilterPtr *tmp_filters = NULL; int nfilters = 0; @@ -594,7 +601,8 @@ cleanup: static int -nwfilterUndefine(virNWFilterPtr obj) { +nwfilterUndefine(virNWFilterPtr obj) +{ virNWFilterDriverStatePtr driver = obj-conn-nwfilterPrivateData; virNWFilterObjPtr nwfilter; int ret = -1; @@ -682,7 +690,8 @@ nwfilterInstantiateFilter(virConnectPtr conn, static void -nwfilterTeardownFilter(virDomainNetDefPtr net) { +nwfilterTeardownFilter(virDomainNetDefPtr net) +{ if ((net-ifname) (net-filter)) virNWFilterTeardownFilter(net); } @@ -717,7 +726,8 @@ static virDomainConfNWFilterDriver domainNWFilterDriver = { }; -int nwfilterRegister(void) { +int nwfilterRegister(void) +{ if (virRegisterNWFilterDriver(nwfilterDriver) 0) return -1; if (virRegisterStateDriver(stateDriver) 0) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 6909a9a..e535356 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3198,7 +3198,8 @@ ebiptablesInstCommand(virBufferPtr buf, * In case of this driver we need the ebtables tool available. */ static int -ebiptablesCanApplyBasicRules(void) { +ebiptablesCanApplyBasicRules(void) +{ return ebtables_cmd_path != NULL; } diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index e156a93..e01d4df 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -2,7 +2,7 @@ * nwfilter_learnipaddr.c: support for learning IP address used by a VM * on an interface * - * Copyright (C) 2011, 2013 Red Hat, Inc. + * Copyright (C) 2011, 2013, 2014 Red Hat, Inc. * Copyright (C) 2010 IBM Corp. * Copyright (C) 2010 Stefan Berger * @@ -138,7 +138,8 @@ static bool threadsTerminate = false; int -virNWFilterLockIface(const char *ifname) { +virNWFilterLockIface(const char *ifname) +{ virNWFilterIfaceLockPtr ifaceLock; virMutexLock(ifaceMapLock); @@ -188,13 +189,15 @@
[libvirt] [PATCH v2 10/16] Use KR style for curly braces in src/test/test_driver.c
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/test/test_driver.c | 135 - 1 file changed, 90 insertions(+), 45 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5716449..d88d3fc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -319,7 +319,8 @@ testBuildXMLConfig(void) static virCapsPtr -testBuildCapabilities(virConnectPtr conn) { +testBuildCapabilities(virConnectPtr conn) +{ testConnPtr privconn = conn-privateData; virCapsPtr caps; virCapsGuestPtr guest; @@ -504,7 +505,8 @@ testDomObjFromDomain(virDomainPtr domain) } static char * -testDomainGenerateIfname(virDomainDefPtr domdef) { +testDomainGenerateIfname(virDomainDefPtr domdef) +{ int maxif = 1024; int ifctr; size_t i; @@ -851,7 +853,8 @@ error: static char *testBuildFilename(const char *relativeTo, - const char *filename) { + const char *filename) +{ char *offset; int baseLen; char *ret; @@ -2495,7 +2498,8 @@ static char *testDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) { return ret; } -static unsigned long long testDomainGetMaxMemory(virDomainPtr domain) { +static unsigned long long testDomainGetMaxMemory(virDomainPtr domain) +{ testConnPtr privconn = domain-conn-privateData; virDomainObjPtr privdom; unsigned long long ret = 0; @@ -2898,7 +2902,8 @@ cleanup: return ret; } -static int testConnectNumOfDefinedDomains(virConnectPtr conn) { +static int testConnectNumOfDefinedDomains(virConnectPtr conn) +{ testConnPtr privconn = conn-privateData; int count; @@ -2911,7 +2916,8 @@ static int testConnectNumOfDefinedDomains(virConnectPtr conn) { static int testConnectListDefinedDomains(virConnectPtr conn, char **const names, - int maxnames) { + int maxnames) +{ testConnPtr privconn = conn-privateData; int n; @@ -2926,7 +2932,8 @@ static int testConnectListDefinedDomains(virConnectPtr conn, } static virDomainPtr testDomainDefineXML(virConnectPtr conn, -const char *xml) { +const char *xml) +{ testConnPtr privconn = conn-privateData; virDomainPtr ret = NULL; virDomainDefPtr def; @@ -3040,7 +3047,8 @@ cleanup: static int testNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freemems, - int startCell, int maxCells) { + int startCell, int maxCells) +{ testConnPtr privconn = conn-privateData; int cell; size_t i; @@ -3066,7 +3074,8 @@ cleanup: } -static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { +static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) +{ testConnPtr privconn = domain-conn-privateData; virDomainObjPtr privdom; virObjectEventPtr event = NULL; @@ -3108,7 +3117,8 @@ cleanup: return ret; } -static int testDomainCreate(virDomainPtr domain) { +static int testDomainCreate(virDomainPtr domain) +{ return testDomainCreateWithFlags(domain, 0); } @@ -3475,7 +3485,8 @@ static virDrvOpenStatus testNetworkOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int testNetworkClose(virConnectPtr conn) { +static int testNetworkClose(virConnectPtr conn) +{ conn-networkPrivateData = NULL; return 0; } @@ -3530,7 +3541,8 @@ cleanup: } -static int testConnectNumOfNetworks(virConnectPtr conn) { +static int testConnectNumOfNetworks(virConnectPtr conn) +{ testConnPtr privconn = conn-privateData; int numActive = 0; size_t i; @@ -3574,7 +3586,8 @@ error: return -1; } -static int testConnectNumOfDefinedNetworks(virConnectPtr conn) { +static int testConnectNumOfDefinedNetworks(virConnectPtr conn) +{ testConnPtr privconn = conn-privateData; int numInactive = 0; size_t i; @@ -3678,7 +3691,8 @@ cleanup: } -static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml) { +static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml) +{ testConnPtr privconn = conn-privateData; virNetworkDefPtr def; virNetworkObjPtr net = NULL; @@ -3744,7 +3758,8 @@ cleanup: return ret; } -static int testNetworkUndefine(virNetworkPtr network) { +static int testNetworkUndefine(virNetworkPtr network) +{ testConnPtr privconn = network-conn-privateData; virNetworkObjPtr privnet; int ret = -1; @@ -3831,7 +3846,8 @@ cleanup: return ret; } -static int testNetworkCreate(virNetworkPtr network) { +static int testNetworkCreate(virNetworkPtr network) +{ testConnPtr privconn = network-conn-privateData; virNetworkObjPtr privnet; int
[libvirt] [PATCH v2 07/16] Use KR style for curly braces in src/storage/
Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/storage/storage_backend_fs.c | 12 --- src/storage/storage_driver.c | 78 ++-- 2 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index edb7cd0..722193b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -330,7 +330,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE * Return 0 if not mounted, 1 if mounted, -1 on error */ static int -virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { +virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) +{ FILE *mtab; struct mntent ent; char buf[1024]; @@ -363,7 +364,8 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { * Returns 0 if successfully mounted, -1 on error */ static int -virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { +virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) +{ char *src = NULL; /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), * while plain 'mount' does. We have to craft separate argvs to @@ -467,7 +469,8 @@ cleanup: * Returns 0 if successfully unmounted, -1 on error */ static int -virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { +virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) +{ virCommandPtr cmd = NULL; int ret = -1; int rc; @@ -562,7 +565,8 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, #if WITH_BLKID static virStoragePoolProbeResult virStorageBackendFileSystemProbe(const char *device, - const char *format) { + const char *format) +{ virStoragePoolProbeResult ret = FILESYSTEM_PROBE_ERROR; blkid_probe probe = NULL; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6a2840c..3637aa5 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -68,7 +68,8 @@ static void storageDriverUnlock(virStorageDriverStatePtr driver) } static void -storageDriverAutostart(virStorageDriverStatePtr driver) { +storageDriverAutostart(virStorageDriverStatePtr driver) +{ size_t i; virConnectPtr conn = NULL; @@ -217,7 +218,8 @@ storageStateAutoStart(void) * files and update its state */ static int -storageStateReload(void) { +storageStateReload(void) +{ if (!driverState) return -1; @@ -238,7 +240,8 @@ storageStateReload(void) { * Shutdown the storage driver, it will stop all active storage pools */ static int -storageStateCleanup(void) { +storageStateCleanup(void) +{ if (!driverState) return -1; @@ -260,7 +263,8 @@ storageStateCleanup(void) { static virStoragePoolPtr storagePoolLookupByUUID(virConnectPtr conn, -const unsigned char *uuid) { +const unsigned char *uuid) +{ virStorageDriverStatePtr driver = conn-storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; @@ -289,7 +293,8 @@ cleanup: static virStoragePoolPtr storagePoolLookupByName(virConnectPtr conn, -const char *name) { +const char *name) +{ virStorageDriverStatePtr driver = conn-storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; @@ -317,7 +322,8 @@ cleanup: } static virStoragePoolPtr -storagePoolLookupByVolume(virStorageVolPtr vol) { +storagePoolLookupByVolume(virStorageVolPtr vol) +{ virStorageDriverStatePtr driver = vol-conn-storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; @@ -359,13 +365,15 @@ storageOpen(virConnectPtr conn, } static int -storageClose(virConnectPtr conn) { +storageClose(virConnectPtr conn) +{ conn-storagePrivateData = NULL; return 0; } static int -storageConnectNumOfStoragePools(virConnectPtr conn) { +storageConnectNumOfStoragePools(virConnectPtr conn) +{ virStorageDriverStatePtr driver = conn-storagePrivateData; size_t i; int nactive = 0; @@ -390,7 +398,8 @@ storageConnectNumOfStoragePools(virConnectPtr conn) { static int storageConnectListStoragePools(virConnectPtr conn, char **const names, - int nnames) { + int nnames) +{ virStorageDriverStatePtr driver = conn-storagePrivateData; int got = 0; size_t i; @@ -424,7 +433,8 @@ storageConnectListStoragePools(virConnectPtr conn, } static int -storageConnectNumOfDefinedStoragePools(virConnectPtr conn) { +storageConnectNumOfDefinedStoragePools(virConnectPtr conn) +{ virStorageDriverStatePtr driver = conn-storagePrivateData; size_t i; int nactive = 0; @@ -449,7 +459,8 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn) { static
Re: [libvirt] [PATCH] build: Fix build with old automake
On Tue, Mar 18, 2014 at 09:44:43 -0600, Eric Blake wrote: On 03/18/2014 09:32 AM, Jiri Denemark wrote: On Tue, Mar 18, 2014 at 06:27:48 -0600, Eric Blake wrote: On 03/18/2014 05:00 AM, Jiri Denemark wrote: Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule: $(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@ be just changed to cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@ That's what I tried first but it does not work at all. I don't understand why but make thinks cpu/cpu_map.xml target is uptodate even though the file does not exist in builddir. That would be VPATH rewriting at play. Does $(builddir)/cpu/cpu_map.xml fare any better? No, that doesn't work either. ... But maybe that's some food for thought - instead of having a rule that uses a direct file name, perhaps you can instead have a witness rule on a stamp file name, where we write the link ourselves when needed, and then have all dependencies be on the stamp (which will ALWAYS exist only in builddir): cpu/cpu_map.xml.stamp: $(AM_V_GEN)if test -f cpu/cpu_map.xml; then \ :; else \ ln -s `cd $(srcdir) pwd`/cpu/cpu_map.xml \ cpu/cpu_map.xml;\ fi touch $@ OK, this seems to work. It's uglier and doesn't regenerate the link if someone removes it (the *.stamp file would need to be removed too) but if that's considered a better way compared to setting abs_*dir, I can make a proper patch out of it after testing it in all situations. Everything we've tried is ugly. At this point, relying on the GNU make extension of $(shell), and using = instead of ?=, is probably the least bad solution. So feel free to keep testing the .stamp alternative if you want, but I'm okay if you check in what you've already tested instead of sinking more time into it. Oh, one other thing I thought of: Why not just name the git version cpu_map.xml.in, and use autoconf's normal mechanisms to always create the non-.in version in builddir. True, there's no @foo@ sequences being replaced, but that way, we don't have to worry about $(abs_srcdir) in the makefile at all; we also don't have to mess with a .stamp. OK, I went ahead and pushed the minimal solution, i.e., setting abs_{src,build}dir = $(shell ...) After all, having the variables set may save us from similar build issues should any of the abs variables be useful for other rules. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Fix the test for the result of atomic dec and test
On Tue, Mar 18, 2014 at 09:45:14PM -0400, Stefan Berger wrote: From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Use virAtomicIntGet() to test for 0 rather than trying to test for 'true' after virAtomicIntDecAndTest(). Avoid putting of the virNWFilterSnoopReq once the thread has been started. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..32ca304 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -720,7 +720,10 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req) virNWFilterSnoopLock(); -if (virAtomicIntDecAndTest(req-refctr)) { +virAtomicIntDecAndTest(req-refctr); + +/* make sure it's 0; virAtomitIntDecAndTest may return true on '1' */ +if (virAtomicIntGet(req-refctr) == 0) { NACK, using two atomic functions, you are making this non-atomic. between these two atomic operations many things can happen an it's not what you want I bet. The virAtomicIntDecAndTest() uses __sync_fetch_and_sub(atomic, 1) and compares it to 1, that's true, but since it is fetch_and_sub (and not the other way around, the value being compared to 1 is the value that the atomic had before it was decremented. That means it returns 1 (true) if and only if the current value is 0. virAtomicIntDecAndTest() is what you really want and should use here. Martin /* * delete the request: * - if we don't find req on the global list anymore @@ -1605,6 +1608,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; +bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1690,6 +1694,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, /* prevent thread from holding req */ virNWFilterSnoopReqLock(req); +threadPuts = true; + if (virThreadCreate(thread, false, virNWFilterDHCPSnoopThread, req) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1737,7 +1743,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: -virNWFilterSnoopReqPut(req); +if (!threadPuts) +virNWFilterSnoopReqPut(req); return -1; } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Fix the test for the result of atomic dec and test
Martin Kletzander mklet...@redhat.com wrote on 03/19/2014 09:13:51 AM: On Tue, Mar 18, 2014 at 09:45:14PM -0400, Stefan Berger wrote: From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Use virAtomicIntGet() to test for 0 rather than trying to test for 'true' after virAtomicIntDecAndTest(). Avoid putting of the virNWFilterSnoopReq once the thread has been started. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/ nwfilter_dhcpsnoop.c index d2a8062..32ca304 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -720,7 +720,10 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req) virNWFilterSnoopLock(); -if (virAtomicIntDecAndTest(req-refctr)) { +virAtomicIntDecAndTest(req-refctr); + +/* make sure it's 0; virAtomitIntDecAndTest may return true on '1' */ +if (virAtomicIntGet(req-refctr) == 0) { NACK, using two atomic functions, you are making this non-atomic. between these two atomic operations many things can happen an it's not what you want I bet. The virAtomicIntDecAndTest() uses __sync_fetch_and_sub(atomic, 1) and compares it to 1, that's true, but since it is fetch_and_sub (and not the other way around, the value being compared to 1 is the value that the atomic had before it was decremented. That means it returns 1 (true) if and only if the current value is 0. virAtomicIntDecAndTest() is what you really want and should use here. Right. I initially thought there was something wrong in this part here until I found the double free error is related to the other part. So this part you comment on doesn't need to be changed. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter: Fix double free of pointer
From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..6a2de98 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1605,6 +1605,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; +bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1690,6 +1691,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, /* prevent thread from holding req */ virNWFilterSnoopReqLock(req); +threadPuts = true; + if (virThreadCreate(thread, false, virNWFilterDHCPSnoopThread, req) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1737,7 +1740,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: -virNWFilterSnoopReqPut(req); +if (!threadPuts) +virNWFilterSnoopReqPut(req); return -1; } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Fix make distcheck
I forgot to delete the underscore in object_locking_SOURCES when changing the name in one of previous cleanups. Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: Pushed as a build-breaker. tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index f80e7ad..90f70ff 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1016,7 +1016,7 @@ CILOPTINCS = CILOPTPACKAGES = -package unix,str,cil CILOPTLIBS = -linkpkg -object_locking_SOURCES = objectlocking.ml +objectlocking_SOURCES = objectlocking.ml %.cmx: %.ml ocamlfind ocamlopt $(CILOPTFLAGS) $(CILOPTINCS) $(CILOPTPACKAGES) -c $ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Set abs_*dir in a consistent way
Use $(shell cd $(...) pwd) to set abs_*dir variables similarly to what src/Makefile.am does. Signed-off-by: Jiri Denemark jdene...@redhat.com --- tests/Makefile.am | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 90f70ff..20a98c7 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -16,6 +16,10 @@ ## License along with this library. If not, see ## http://www.gnu.org/licenses/. +# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) pwd) + SHELL = $(PREFERABLY_POSIX_SHELL) INCLUDES = \ @@ -28,8 +32,8 @@ INCLUDES = \ $(GETTEXT_CPPFLAGS) AM_CFLAGS = \ - -Dabs_builddir=\`pwd`\ \ - -Dabs_srcdir=\`cd '$(srcdir)'; pwd`\ \ + -Dabs_builddir=\$(abs_builddir)\ \ + -Dabs_srcdir=\$(abs_srcdir)\ \ $(LIBXML_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ @@ -373,7 +377,7 @@ TESTS = $(test_programs) \ # Also, BSD sh doesn't like 'a=b b=$$a', so we can't use an # intermediate shell variable, but must do all the expansion in make -lv_abs_top_builddir=`cd '$(top_builddir)'; pwd` +lv_abs_top_builddir=$(shell cd '$(top_builddir)'; pwd) path_add = $(subst :,$(PATH_SEPARATOR),\ $(subst !,$(lv_abs_top_builddir)/,!daemon:!tools:!tests)) @@ -381,9 +385,9 @@ VIR_TEST_EXPENSIVE ?= $(VIR_TEST_EXPENSIVE_DEFAULT) TESTS_ENVIRONMENT =\ abs_top_builddir=$(lv_abs_top_builddir) \ abs_top_srcdir=`cd '$(top_srcdir)'; pwd` \ - abs_builddir=`pwd` \ - abs_srcdir=`cd '$(srcdir)'; pwd` \ - CONFIG_HEADER=`cd '$(top_builddir)'; pwd`/config.h \ + abs_builddir=$(abs_builddir) \ + abs_srcdir=$(abs_srcdir) \ + CONFIG_HEADER=$(lv_abs_top_builddir)/config.h \ PATH=$(path_add)$(PATH_SEPARATOR)$$PATH\ SHELL=$(SHELL) \ LIBVIRT_DRIVER_DIR=$(lv_abs_top_builddir)/src/.libs \ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Make sure src/util/virprobe.h is distributed
Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Makefile.am b/src/Makefile.am index 5ff178b..4fdd871 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -137,6 +137,7 @@ UTIL_SOURCES = \ util/virpci.c util/virpci.h \ util/virpidfile.c util/virpidfile.h \ util/virportallocator.c util/virportallocator.h \ + util/virprobe.h \ util/virprocess.c util/virprocess.h \ util/virrandom.h util/virrandom.c \ util/virscsi.c util/virscsi.h \ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] [PATCH] Fix flawed testcases in qemuhotplugtest.c
On 19.03.2014 00:08, Nehal J Wani wrote: While running qemuhotplugtest, it was found that valgrind pointed out the following memory leak: ==7906== 5 bytes in 1 blocks are definitely lost in loss record 7 of 121 ==7906==at 0x4A069EE: malloc (vg_replace_malloc.c:270) ==7906==by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6) ==7906==by 0x4CDAE03: virDomainDeviceInfoParseXML.isra.32 (domain_conf.c:3685) ==7906==by 0x4CE3BB9: virDomainNetDefParseXML (domain_conf.c:6707) ==7906==by 0x4CFBA08: virDomainDefParseXML (domain_conf.c:12235) ==7906==by 0x4CFBC1E: virDomainDefParseNode (domain_conf.c:13039) ==7906==by 0x4CFBD95: virDomainDefParse (domain_conf.c:12981) ==7906==by 0x41FEB4: testQemuHotplug (qemuhotplugtest.c:66) ==7906==by 0x420F41: virtTestRun (testutils.c:201) ==7906==by 0x41F287: mymain (qemuhotplugtest.c:422) ==7906==by 0x4216BD: virtTestMain (testutils.c:784) ==7906==by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ...and 10 more. The 'alias' attribute should *not* be parsed from the XML provided by the user. It should only be parsed in the live state XML. In the latter case no codepath should take us to qemuAssignDeviceAliases (which, in this case, is causing the above leaks). You're right, in case of domains, alias should not be parsed from user supplied XMLs. But the XMLs used in the test are not user supplied ones. In fact, we want domain to contain all information that a running domain does (e.g. all aliases) - so for the hotplug code it looks like domain is running (coldplug code just don't care). Having said that, I agree that there's no need to have aliases in the XMLs but generate them on the fly. That's what is happening now, well since 20745748 which introduced the generation at runtime. So I guess your comment is not quite correct. --- Caused by the test cases: DO_TEST_ATTACH(console-compat-2, console-virtio, false, true, chardev-add, {\return\: {\pty\: \/dev/pts/26\}}, device_add, QMP_OK); DO_TEST_DETACH(console-compat-2, console-virtio, false, false, device_del, QMP_OK, chardev-remove, QMP_OK); Refer: http://www.redhat.com/archives/libvir-list/2013-November/msg01187.html ...qemuhotplug-console-compat-2+console-virtio.xml | 34 +++- .../qemuhotplug-console-virtio.xml |1 - .../qemuxml2argv-console-compat-2.xml | 32 +++--- 3 files changed, 10 insertions(+), 57 deletions(-) The patch is incomplete. We need this as well: diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 8036adc..4ef81e0 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -67,7 +67,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, driver.caps, driver.xmlopt, QEMU_EXPECTED_VIRT_TYPES, - 0))) + VIR_DOMAIN_XML_INACTIVE))) goto cleanup; priv = (*vm)-privateData; Anyway, I would not call the testcases flawed, just mem-leaking. There's still one mem-leak that I'll investigate further. ACK with the qemuhotplugtest change squashed in. I'm rewording both the subject and commit message and pushing this in. Thanks for sorting this out. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: Make sure src/util/virprobe.h is distributed
On Wed, Mar 19, 2014 at 02:48:33PM +0100, Jiri Denemark wrote: Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Makefile.am b/src/Makefile.am index 5ff178b..4fdd871 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -137,6 +137,7 @@ UTIL_SOURCES = \ util/virpci.c util/virpci.h \ util/virpidfile.c util/virpidfile.h \ util/virportallocator.c util/virportallocator.h \ + util/virprobe.h \ util/virprocess.c util/virprocess.h \ util/virrandom.h util/virrandom.c \ util/virscsi.c util/virscsi.h \ ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: consistent comments about disk enum usage
Before refactoring this struct, I found it helpful to track which 'int' fields really contain an enum value. * src/conf/domain_conf.h (_virDomainDiskDef): Add comments. Signed-off-by: Eric Blake ebl...@redhat.com --- Pushing under the trivial rule. src/conf/domain_conf.h | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 37191a8..27f07e6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -494,7 +494,7 @@ struct _virDomainHostdevDef { virDomainDeviceInfoPtr info; /* Guest address */ }; -/* Two types of disk backends */ +/* Types of disk backends (host resource) */ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, VIR_DOMAIN_DISK_TYPE_FILE, @@ -505,7 +505,7 @@ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_LAST }; -/* Three types of disk frontend */ +/* Types of disk frontend (guest view) */ enum virDomainDiskDevice { VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM, @@ -529,7 +529,7 @@ enum virDomainDiskBus { VIR_DOMAIN_DISK_BUS_LAST }; -enum virDomainDiskCache { +enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_DEFAULT, VIR_DOMAIN_DISK_CACHE_DISABLE, VIR_DOMAIN_DISK_CACHE_WRITETHRU, @@ -540,7 +540,7 @@ enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_LAST }; -enum virDomainDiskErrorPolicy { +enum virDomainDiskErrorPolicy { VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT, VIR_DOMAIN_DISK_ERROR_POLICY_STOP, VIR_DOMAIN_DISK_ERROR_POLICY_REPORT, @@ -580,7 +580,7 @@ enum virDomainDiskTray { VIR_DOMAIN_DISK_TRAY_LAST }; -enum virDomainDiskGeometryTrans { +enum virDomainDiskGeometryTrans { VIR_DOMAIN_DISK_TRANS_DEFAULT = 0, VIR_DOMAIN_DISK_TRANS_NONE, VIR_DOMAIN_DISK_TRANS_AUTO, @@ -598,7 +598,7 @@ struct _virDomainDiskHostDef { char *socket; /* path to unix socket */ }; -enum virDomainDiskIo { +enum virDomainDiskIo { VIR_DOMAIN_DISK_IO_DEFAULT, VIR_DOMAIN_DISK_IO_NATIVE, VIR_DOMAIN_DISK_IO_THREADS, @@ -707,14 +707,14 @@ typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; /* Stores the virtual disk configuration */ struct _virDomainDiskDef { -int type; -int device; -int bus; +int type; /* enum virDomainDiskType */ +int device; /* enum virDomainDiskDevice */ +int bus; /* enum virDomainDiskBus */ char *src; char *dst; -int tray_status; -int removable; -int protocol; +int tray_status; /* enum virDomainDiskTray */ +int removable; /* enum virDomainFeatureState */ +int protocol; /* enum virDomainDiskProtocol */ size_t nhosts; virDomainDiskHostDefPtr hosts; virDomainDiskSourcePoolDefPtr srcpool; @@ -738,7 +738,7 @@ struct _virDomainDiskDef { unsigned int cylinders; unsigned int heads; unsigned int sectors; -int trans; +int trans; /* enum virDomainDiskGeometryTrans */ } geometry; struct { @@ -752,13 +752,13 @@ struct _virDomainDiskDef { char *wwn; char *vendor; char *product; -int cachemode; +int cachemode; /* enum virDomainDiskCache */ int error_policy; /* enum virDomainDiskErrorPolicy */ int rerror_policy; /* enum virDomainDiskErrorPolicy */ -int iomode; -int ioeventfd; -int event_idx; -int copy_on_read; +int iomode; /* enum virDomainDiskIo */ +int ioeventfd; /* enum virDomainIoEventFd */ +int event_idx; /* enum virDomainVirtioEventIdx */ +int copy_on_read; /* enum virDomainDiskCopyOnRead */ int snapshot; /* enum virDomainSnapshotLocation, snapshot_conf.h */ int startupPolicy; /* enum virDomainStartupPolicy */ bool readonly; -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuDomainAttachDeviceFlags: Parse device xml as inactive
In all other drivers we are doing so. Moreover, we don't want to parse runtime information in attach (even if the attach is meant as live) because we are generating the runtime info ourselves. We can't trust users they supply sane values anyway. ==1140== 9 bytes in 1 blocks are definitely lost in loss record 72 of 1,151 ==1140==at 0x4A06C2B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==1140==by 0x623C758: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1) ==1140==by 0x50FD763: virXMLPropString (virxml.c:483) ==1140==by 0x510F8B7: virDomainDeviceInfoParseXML (domain_conf.c:3685) ==1140==by 0x511ACFD: virDomainChrDefParseXML (domain_conf.c:7535) ==1140==by 0x5121D13: virDomainDeviceDefParse (domain_conf.c:9918) ==1140==by 0x13AE6313: qemuDomainAttachDeviceFlags (qemu_driver.c:6926) ==1140==by 0x13AE65FA: qemuDomainAttachDevice (qemu_driver.c:7005) ==1140==by 0x51C77DA: virDomainAttachDevice (libvirt.c:10231) ==1140==by 0x127FDD: remoteDispatchDomainAttachDevice (remote_dispatch.h:2404) ==1140==by 0x127EC5: remoteDispatchDomainAttachDeviceHelper (remote_dispatch.h:2382) ==1140==by 0x5241F81: virNetServerProgramDispatchCall (virnetserverprogram.c:437) When doing live attach, we are passing the inactive definition anyway since we are passing the result of virDomainDeviceDefCopy() which does inactive copy by default. Moreover, we are doing the same mistake in qemuhotplugtest. Just a side note - it makes perfect sense to parse the runtime info like alias in qemuDomainDetachDevice and qemuDomainUpdateDeviceFlags() as in some cases the only difference to distinguish two devices can be just their alias. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_driver.c | 6 +- tests/qemuhotplugtest.c | 7 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2707bec..8678d24 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6871,7 +6871,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virDomainDefPtr vmdef = NULL; virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; int ret = -1; -unsigned int affect, parse_flags = 0; +unsigned int affect, parse_flags = VIR_DOMAIN_XML_INACTIVE; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; @@ -6919,10 +6919,6 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; } -if ((flags VIR_DOMAIN_AFFECT_CONFIG) -!(flags VIR_DOMAIN_AFFECT_LIVE)) -parse_flags |= VIR_DOMAIN_XML_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm-def, caps, driver-xmlopt, parse_flags); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 4ef81e0..a7a4065 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -209,6 +209,7 @@ testQemuHotplug(const void *data) const char *const *tmp; bool fail = test-fail; bool keep = test-keep; +unsigned int device_parse_flags = 0; virDomainObjPtr vm = NULL; virDomainDeviceDefPtr dev = NULL; virCapsPtr caps = NULL; @@ -244,8 +245,12 @@ testQemuHotplug(const void *data) goto cleanup; } +if (test-action == ATTACH) +device_parse_flags = VIR_DOMAIN_XML_INACTIVE; + if (!(dev = virDomainDeviceDefParse(device_xml, vm-def, -caps, driver.xmlopt, 0))) +caps, driver.xmlopt, +device_parse_flags))) goto cleanup; /* Now is the best time to feed the spoofed monitor with predefined -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] daemon: Enhance documentation for changing NOFILE limit
On 03/19/2014 09:32 AM, Jiri Denemark wrote: Signed-off-by: Jiri Denemark jdene...@redhat.com --- daemon/libvirtd.sysconf | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/daemon/libvirtd.sysconf b/daemon/libvirtd.sysconf index 3af1f03..8b3 100644 --- a/daemon/libvirtd.sysconf +++ b/daemon/libvirtd.sysconf @@ -20,5 +20,13 @@ # #SDL_AUDIODRIVER=pulse -# Override the maximum number of opened files +# Override the maximum number of opened files. +# This only works with traditional init scripts. In systemd world, the limit +# can only be changed by overriding LimitNOFILE for libvirtd.service. To do +# that, just create a *.conf file in /etc/systemd/system/libvirtd.service.d/ +# (for example /etc/systemd/system/libvirtd.service.d/openfiles.conf) and +# write the following two lines in it: +# [Service] +# LimitNOFILE=2048 +# #LIBVIRTD_NOFILES_LIMIT=2048 ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] daemon: Enhance documentation for changing NOFILE limit
Signed-off-by: Jiri Denemark jdene...@redhat.com --- daemon/libvirtd.sysconf | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/daemon/libvirtd.sysconf b/daemon/libvirtd.sysconf index 3af1f03..8b3 100644 --- a/daemon/libvirtd.sysconf +++ b/daemon/libvirtd.sysconf @@ -20,5 +20,13 @@ # #SDL_AUDIODRIVER=pulse -# Override the maximum number of opened files +# Override the maximum number of opened files. +# This only works with traditional init scripts. In systemd world, the limit +# can only be changed by overriding LimitNOFILE for libvirtd.service. To do +# that, just create a *.conf file in /etc/systemd/system/libvirtd.service.d/ +# (for example /etc/systemd/system/libvirtd.service.d/openfiles.conf) and +# write the following two lines in it: +# [Service] +# LimitNOFILE=2048 +# #LIBVIRTD_NOFILES_LIMIT=2048 -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Fix double free of pointer
On 03/19/2014 09:41 AM, Stefan Berger wrote: From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..6a2de98 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1605,6 +1605,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; +bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1690,6 +1691,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, /* prevent thread from holding req */ virNWFilterSnoopReqLock(req); +threadPuts = true; + So this has to be moved after the virThreadCreate()... Stefan if (virThreadCreate(thread, false, virNWFilterDHCPSnoopThread, req) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1737,7 +1740,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: -virNWFilterSnoopReqPut(req); +if (!threadPuts) +virNWFilterSnoopReqPut(req); return -1; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] nwfilter: Fix double free of pointer
From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..3407604 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1605,6 +1605,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; +bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1698,6 +1699,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, goto exit_snoopreq_unlock; } +threadPuts = true; + virAtomicIntInc(virNWFilterSnoopState.nThreads); req-threadkey = virNWFilterSnoopActivate(req); @@ -1737,7 +1740,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: -virNWFilterSnoopReqPut(req); +if (!threadPuts) +virNWFilterSnoopReqPut(req); return -1; } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/8] Test parsing of iscsiadm output
v1: https://www.redhat.com/archives/libvir-list/2014-March/msg00720.html v2: * instead of using cat, switch RunProgRegex to use a synchronous command and modify the output buffer via a virCommandDryRun callback * move the functions dealing with iscsiadm out of the iscsi backend to src/util (and move StorageBackendRunRegex as well) Ján Tomko (8): Sort includes in storage_backend_iscsi.c Move virStorageBackendRun to vircommand Switch virCommandRunRegex to use virStringSplit Don't create iscsiadm command line in ISCSIPool{Start,Stop} Remove storage pool from the arguments of a few functions Move functions using iscsiadm to viriscsi.c Add test for virISCSIGetSession Add test for virISCSIScanTargets po/POTFILES.in| 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 11 + src/storage/storage_backend.c | 249 - src/storage/storage_backend.h | 22 -- src/storage/storage_backend_disk.c| 43 +-- src/storage/storage_backend_fs.c | 9 +- src/storage/storage_backend_iscsi.c | 482 ++-- src/storage/storage_backend_iscsi.h | 4 - src/storage/storage_backend_logical.c | 63 +++-- src/util/vircommand.c | 242 + src/util/vircommand.h | 20 ++ src/util/viriscsi.c | 498 ++ src/util/viriscsi.h | 52 tests/Makefile.am | 6 + tests/viriscsitest.c | 219 +++ 16 files changed, 1145 insertions(+), 777 deletions(-) create mode 100644 src/util/viriscsi.c create mode 100644 src/util/viriscsi.h create mode 100644 tests/viriscsitest.c -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/8] Sort includes in storage_backend_iscsi.c
--- src/storage/storage_backend_iscsi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 2a4e669..0feeb5f 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -34,13 +34,13 @@ #include datatypes.h #include driver.h -#include virerror.h #include storage_backend_scsi.h #include storage_backend_iscsi.h #include viralloc.h -#include virlog.h -#include virfile.h #include vircommand.h +#include virerror.h +#include virfile.h +#include virlog.h #include virobject.h #include virrandom.h #include virstring.h -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 3/8] Switch virCommandRunRegex to use virStringSplit
Instead of running the command asynchronously and reading the output via fgets, let virCommand collect the output and split it with virStringSplit. --- src/util/vircommand.c | 41 +++-- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 3f98eb8..8250634 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -2779,15 +2779,16 @@ virCommandRunRegex(virCommandPtr cmd, void *data, const char *prefix) { -int fd = -1, err, ret = -1; -FILE *list = NULL; +int err; regex_t *reg; regmatch_t *vars = NULL; -char line[1024]; int maxReg = 0; -size_t i, j; +size_t i, j, k; int totgroups = 0, ngroup = 0, maxvars = 0; char **groups; +char *outbuf = NULL; +char **lines = NULL; +int ret = -1; /* Compile all regular expressions */ if (VIR_ALLOC_N(reg, nregex) 0) @@ -2818,29 +2819,27 @@ virCommandRunRegex(virCommandPtr cmd, if (VIR_ALLOC_N(vars, maxvars+1) 0) goto cleanup; -virCommandSetOutputFD(cmd, fd); -if (virCommandRunAsync(cmd, NULL) 0) { +virCommandSetOutputBuffer(cmd, outbuf); +if (virCommandRun(cmd, NULL) 0) goto cleanup; -} -if ((list = VIR_FDOPEN(fd, r)) == NULL) { -virReportError(VIR_ERR_INTERNAL_ERROR, - %s, _(cannot read fd)); +if (!outbuf) { +/* no output */ +ret = 0; goto cleanup; } -while (fgets(line, sizeof(line), list) != NULL) { +if (!(lines = virStringSplit(outbuf, \n, 0))) +goto cleanup; + +for (k = 0; lines[k]; k++) { char *p = NULL; -/* Strip trailing newline */ -int len = strlen(line); -if (len line[len-1] == '\n') -line[len-1] = '\0'; /* ignore any command prefix */ if (prefix) -p = STRSKIP(line, prefix); +p = STRSKIP(lines[k], prefix); if (!p) -p = line; +p = lines[k]; for (i = 0; i = maxReg i nregex; i++) { if (regexec(reg[i], p, nvars[i]+1, vars, 0) == 0) { @@ -2872,8 +2871,10 @@ virCommandRunRegex(virCommandPtr cmd, } } -ret = virCommandWait(cmd, NULL); +ret = 0; cleanup: +virStringFreeList(lines); +VIR_FREE(outbuf); if (groups) { for (j = 0; j totgroups; j++) VIR_FREE(groups[j]); @@ -2885,10 +2886,6 @@ cleanup: regfree(reg[i]); VIR_FREE(reg); - -VIR_FORCE_FCLOSE(list); -VIR_FORCE_CLOSE(fd); - return ret; } -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Give virNWFilterVarCombIterNext saner semantics
On 03/17/2014 08:34 AM, Daniel P. Berrange wrote: The virNWFilterVarCombIterNext method will free its parameter when it gets to the end of the iterator. This is somewhat misleading design, making it appear as if the caller has a memory leak. Remove the free'ing of the parameter and ensure that the calling method ebiptablesCreateRuleInstanceIterate free's it instead. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/nwfilter_params.c| 4 +--- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index a78c407..5393134 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -526,10 +526,8 @@ next: } } -if (ci-nIter == i) { -virNWFilterVarCombIterFree(ci); +if (ci-nIter == i) return NULL; -} return ci; } diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 0505045..e1e0af8 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2869,14 +2869,14 @@ ebiptablesCreateRuleInstanceIterate( virNWFilterRuleInstPtr res) { int rc = 0; -virNWFilterVarCombIterPtr vciter; +virNWFilterVarCombIterPtr vciter, tmp; /* rule-vars holds all the variables names that this rule will access. * iterate over all combinations of the variables' values and instantiate * the filtering rule with each combination. */ -vciter = virNWFilterVarCombIterCreate(vars, - rule-varAccess, rule-nVarAccess); +tmp = vciter = virNWFilterVarCombIterCreate(vars, +rule-varAccess, rule-nVarAccess); if (!vciter) return -1; @@ -2885,12 +2885,12 @@ ebiptablesCreateRuleInstanceIterate( nwfilter, rule, ifname, - vciter, + tmp, res); if (rc 0) break; -vciter = virNWFilterVarCombIterNext(vciter); -} while (vciter != NULL); +tmp = virNWFilterVarCombIterNext(tmp); +} while (tmp != NULL); virNWFilterVarCombIterFree(vciter); ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix leak on OOM when creating nwfilter rule instances
On 03/17/2014 07:59 AM, Daniel P. Berrange wrote: The ebiptablesAddRuleInst method would leak an instance of ebiptablesRuleInstPtr if it hit OOM when adding it to the list of instances. Remove the pointless helper method virNWFilterRuleInstAddData and just inline the call to VIR_APPEND_ELEMENT and free the instance on failure. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +- src/nwfilter/nwfilter_gentech_driver.c| 22 -- src/nwfilter/nwfilter_gentech_driver.h| 3 --- 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 9dbd3ff..e1e0af8 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -505,7 +505,11 @@ ebiptablesAddRuleInst(virNWFilterRuleInstPtr res, inst-priority = priority; inst-ruleType = ruleType; -return virNWFilterRuleInstAddData(res, inst); +if (VIR_APPEND_ELEMENT(res-data, res-ndata, inst) 0) { +VIR_FREE(inst); +return -1; +} +return 0; } ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 6/8] Move functions using iscsiadm to viriscsi.c
On Wed, Mar 19, 2014 at 04:52:31PM +0100, Ján Tomko wrote: diff --git a/src/util/viriscsi.h b/src/util/viriscsi.h new file mode 100644 index 000..462e56a --- /dev/null +++ b/src/util/viriscsi.h @@ -0,0 +1,52 @@ +#ifndef __VIR_ISCSI_H__ +# define __VIR_ISCSI_H__ + +# include internal.h + +char * +virISCSIGetSession(const char *devpath, + bool probe); Could add ATTRIBUTE_NONNULL for this 'const char *' and all the other pointers below + +int +virISCSIConnectionLogin(const char *portal, +const char *initiatoriqn, +const char *target); +int +virISCSIConnectionLogout(const char *portal, + const char *initiatoriqn, + const char *target); +int +virISCSIRescanLUNs(const char *session); + +int +virISCSIScanTargets(const char *portal, +const char *initiatoriqn, +size_t *ntargetsret, +char ***targetsret); +int +virISCSINodeUpdate(const char *portal, + const char *target, + const char *name, + const char *value); And these 5 could have ATTRIBUTE_RETURN_CHECK too Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/18] conf: use disk source accessors in uml/
Part of a series of cleanups to use new accessor methods. * src/uml/uml_conf.c (umlBuildCommandLine): Use accessors. * src/uml/uml_driver.c (umlDomainAttachUmlDisk): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/uml/uml_conf.c | 4 ++-- src/uml/uml_driver.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 3567b03..27c53f9 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -1,7 +1,7 @@ /* * uml_conf.c: UML driver configuration * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -410,7 +410,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn, goto error; } -virCommandAddArgPair(cmd, disk-dst, disk-src); +virCommandAddArgPair(cmd, disk-dst, virDomainDiskGetSource(disk)); } for (i = 0; i vm-def-nnets; i++) { diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 28e65f4..a49b56b 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2144,13 +2144,14 @@ static int umlDomainAttachUmlDisk(struct uml_driver *driver, } } -if (!disk-src) { +if (!virDomainDiskGetSource(disk)) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(disk source path is missing)); goto error; } -if (virAsprintf(cmd, config %s=%s, disk-dst, disk-src) 0) +if (virAsprintf(cmd, config %s=%s, disk-dst, +virDomainDiskGetSource(disk)) 0) return -1; if (umlMonitorCommand(driver, vm, cmd, reply) 0) -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/18] conf: use disk source accessors in libxl/
Part of a series of cleanups to use new accessor methods. * src/libxl/libxl_conf.c (libxlMakeDisk): Use accessors. * src/libxl/libxl_driver.c (libxlDomainChangeEjectableMedia) (libxlDomainAttachDeviceDiskLive): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/libxl/libxl_conf.c | 50 ++-- src/libxl/libxl_driver.c | 26 ++--- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index cfac847..cf110a3 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1,7 +1,7 @@ /* * libxl_conf.c: libxl configuration management * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * Copyright (c) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany. * Copyright (C) 2011 Univention GmbH. * @@ -716,18 +716,22 @@ error: int libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) { +const char *driver; +int format; + libxl_device_disk_init(x_disk); -if (VIR_STRDUP(x_disk-pdev_path, l_disk-src) 0) +if (VIR_STRDUP(x_disk-pdev_path, virDomainDiskGetSource(l_disk)) 0) return -1; if (VIR_STRDUP(x_disk-vdev, l_disk-dst) 0) return -1; -if (l_disk-driverName) { -if (STREQ(l_disk-driverName, tap) || -STREQ(l_disk-driverName, tap2)) { -switch (l_disk-format) { +driver = virDomainDiskGetDriver(l_disk); +format = virDomainDiskGetFormat(l_disk); +if (driver) { +if (STREQ(driver, tap) || STREQ(driver, tap2)) { +switch (format) { case VIR_STORAGE_FILE_QCOW: x_disk-format = LIBXL_DISK_FORMAT_QCOW; x_disk-backend = LIBXL_DISK_BACKEND_QDISK; @@ -750,13 +754,13 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) virReportError(VIR_ERR_INTERNAL_ERROR, _(libxenlight does not support disk format %s with disk driver %s), - virStorageFileFormatTypeToString(l_disk-format), - l_disk-driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } -} else if (STREQ(l_disk-driverName, qemu)) { +} else if (STREQ(driver, qemu)) { x_disk-backend = LIBXL_DISK_BACKEND_QDISK; -switch (l_disk-format) { +switch (format) { case VIR_STORAGE_FILE_QCOW: x_disk-format = LIBXL_DISK_FORMAT_QCOW; break; @@ -775,30 +779,30 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) virReportError(VIR_ERR_INTERNAL_ERROR, _(libxenlight does not support disk format %s with disk driver %s), - virStorageFileFormatTypeToString(l_disk-format), - l_disk-driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } -} else if (STREQ(l_disk-driverName, file)) { -if (l_disk-format != VIR_STORAGE_FILE_NONE -l_disk-format != VIR_STORAGE_FILE_RAW) { +} else if (STREQ(driver, file)) { +if (format != VIR_STORAGE_FILE_NONE +format != VIR_STORAGE_FILE_RAW) { virReportError(VIR_ERR_INTERNAL_ERROR, _(libxenlight does not support disk format %s with disk driver %s), - virStorageFileFormatTypeToString(l_disk-format), - l_disk-driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; } x_disk-format = LIBXL_DISK_FORMAT_RAW; x_disk-backend = LIBXL_DISK_BACKEND_TAP; -} else if (STREQ(l_disk-driverName, phy)) { -if (l_disk-format != VIR_STORAGE_FILE_NONE -l_disk-format != VIR_STORAGE_FILE_RAW) { +} else if (STREQ(driver, phy)) { +if (format != VIR_STORAGE_FILE_NONE +format != VIR_STORAGE_FILE_RAW) { virReportError(VIR_ERR_INTERNAL_ERROR, _(libxenlight does not support disk format %s with disk driver %s), - virStorageFileFormatTypeToString(l_disk-format), - l_disk-driverName); + virStorageFileFormatTypeToString(format), + driver); return -1; }
[libvirt] [PATCH 11/18] conf: use disk source accessors in security/
Part of a series of cleanups to use new accessor methods. * src/security/security_dac.c (virSecurityDACSetSecurityImageLabel) (virSecurityDACRestoreSecurityImageLabelInt) (virSecurityDACSetSecurityAllLabel): Use accessors. * src/security/security_selinux.c (virSecuritySELinuxRestoreSecurityImageLabelInt) (virSecuritySELinuxSetSecurityImageLabel) (virSecuritySELinuxSetSecurityAllLabel): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/security/security_dac.c | 17 + src/security/security_selinux.c | 18 ++ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9f45063..0bd36b7 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -355,7 +355,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!priv-dynamicOwnership) return 0; -if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) +if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; params[0] = mgr; @@ -374,11 +374,12 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, int migrated) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); +const char *src = virDomainDiskGetSource(disk); if (!priv-dynamicOwnership) return 0; -if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) +if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; /* Don't restore labels on readoly/shared disks, because @@ -392,7 +393,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (disk-readonly || disk-shared) return 0; -if (!disk-src) +if (!src) return 0; /* If we have a shared FS doing migrated, we must not @@ -401,17 +402,17 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * VM's I/O attempts :-) */ if (migrated) { -int rc = virStorageFileIsSharedFS(disk-src); +int rc = virStorageFileIsSharedFS(src); if (rc 0) return -1; if (rc == 1) { VIR_DEBUG(Skipping image label restore on %s because FS is shared, - disk-src); + src); return 0; } } -return virSecurityDACRestoreSecurityFileLabel(disk-src); +return virSecurityDACRestoreSecurityFileLabel(src); } @@ -904,7 +905,7 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, for (i = 0; i def-ndisks; i++) { /* XXX fixme - we need to recursively label the entire tree :-( */ -if (def-disks[i]-type == VIR_DOMAIN_DISK_TYPE_DIR) +if (virDomainDiskGetType(def-disks[i]) == VIR_DOMAIN_DISK_TYPE_DIR) continue; if (virSecurityDACSetSecurityImageLabel(mgr, def, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 02c7496..32f6c7d 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2013 Red Hat, Inc. + * Copyright (C) 2008-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1133,6 +1133,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, { virSecurityLabelDefPtr seclabel; virSecurityDeviceLabelDefPtr disk_seclabel; +const char *src = virDomainDiskGetSource(disk); seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (seclabel == NULL) @@ -1162,7 +1163,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, if (disk-readonly || disk-shared) return 0; -if (!disk-src || disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) +if (!src || virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; /* If we have a shared FS doing migrated, we must not @@ -1171,17 +1172,17 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, * VM's I/O attempts :-) */ if (migrated) { -int rc = virStorageFileIsSharedFS(disk-src); +int rc = virStorageFileIsSharedFS(src); if (rc 0) return -1; if (rc == 1) { VIR_DEBUG(Skipping image label restore on %s because FS is shared, - disk-src); + src); return 0; } } -return virSecuritySELinuxRestoreSecurityFileLabel(mgr, disk-src); +return
[libvirt] [PATCH 00/18] split virDomainDiskDef
I still have quite a bit of work to consolidate util/virStorageFile, conf/snapshot_conf.c, and the new struct in this series to all share the same common struct (basically, moving the struct created in this series over to util). But by refactoring the most common accesses to go through functions instead of direct field access, I've limited the amount of code that is actually impacted by using the new struct. In general, this series should not be changing any behavior. Eric Blake (18): conf: accessors for common source information conf: use disk source accessors in conf/ conf: use disk source accessors in bhyve/ conf: use disk source accessors in esx/ conf: use disk source accessors in libxl/ conf: use disk source accessors in locking/ conf: use disk source accessors in lxc/ conf: use disk source accessors in parallels/ conf: use disk source accessors in phyp/ conf: use disk source accessors in qemu/ conf: use disk source accessors in security/ conf: use disk source accessors in uml/ conf: use disk source accessors in vbox/ conf: use disk source accessors in vmware/ conf: use disk source accessors in vmx/ conf: use disk source accessors in xen/ conf: use disk source accessors in xenxs/ conf: prepare to track multiple host source files per disk src/bhyve/bhyve_command.c| 9 +- src/conf/domain_audit.c | 7 +- src/conf/domain_conf.c | 253 +--- src/conf/domain_conf.h | 47 -- src/conf/snapshot_conf.c | 2 +- src/esx/esx_driver.c | 26 ++-- src/libvirt_private.syms | 8 + src/libxl/libxl_conf.c | 50 --- src/libxl/libxl_driver.c | 26 ++-- src/locking/domain_lock.c| 19 ++- src/lxc/lxc_cgroup.c | 6 +- src/lxc/lxc_controller.c | 77 +- src/lxc/lxc_driver.c | 35 +++-- src/parallels/parallels_driver.c | 31 ++-- src/phyp/phyp_driver.c | 10 +- src/qemu/qemu_command.c | 308 --- src/qemu/qemu_conf.c | 108 +++--- src/qemu/qemu_domain.c | 60 src/qemu/qemu_driver.c | 203 +- src/qemu/qemu_hotplug.c | 116 --- src/qemu/qemu_migration.c| 21 +-- src/qemu/qemu_process.c | 19 +-- src/security/security_dac.c | 17 ++- src/security/security_selinux.c | 20 +-- src/storage/storage_driver.c | 8 +- src/uml/uml_conf.c | 4 +- src/uml/uml_driver.c | 5 +- src/vbox/vbox_tmpl.c | 137 + src/vmware/vmware_conf.c | 18 ++- src/vmware/vmware_conf.h | 6 +- src/vmx/vmx.c| 113 -- src/xen/xend_internal.c | 11 +- src/xenxs/xen_sxpr.c | 111 +++--- src/xenxs/xen_xm.c | 113 +++--- tests/securityselinuxlabeltest.c | 8 +- 35 files changed, 1119 insertions(+), 893 deletions(-) -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/18] conf: use disk source accessors in esx/
Part of a series of cleanups to use new accessor methods. * src/esx/esx_driver.c (esxAutodetectSCSIControllerModel) (esxDomainDefineXML): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- src/esx/esx_driver.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index a0591f3..a08a69d 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1,7 +1,7 @@ /* * esx_driver.c: core driver functions for managing VMware ESX hosts * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2009-2013 Matthias Bolte matthias.bo...@googlemail.com * Copyright (C) 2009 Maximilian Wilhelm m...@rfc2324.org * @@ -385,12 +385,12 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, esxVMX_Data *data = opaque; esxVI_FileInfo *fileInfo = NULL; esxVI_VmDiskFileInfo *vmDiskFileInfo = NULL; +const char *src = virDomainDiskGetSource(def); if (def-device != VIR_DOMAIN_DISK_DEVICE_DISK || def-bus != VIR_DOMAIN_DISK_BUS_SCSI || -def-type != VIR_DOMAIN_DISK_TYPE_FILE || -!def-src || -! STRPREFIX(def-src, [)) { +virDomainDiskGetType(def) != VIR_DOMAIN_DISK_TYPE_FILE || +!src || !STRPREFIX(src, [)) { /* * This isn't a file-based SCSI disk device with a datastore related * source path = do nothing. @@ -398,7 +398,7 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, return 0; } -if (esxVI_LookupFileInfoByDatastorePath(data-ctx, def-src, +if (esxVI_LookupFileInfoByDatastorePath(data-ctx, src, false, fileInfo, esxVI_Occurrence_RequiredItem) 0) { goto cleanup; @@ -408,7 +408,7 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, if (!vmDiskFileInfo || !vmDiskFileInfo-controllerType) { virReportError(VIR_ERR_INTERNAL_ERROR, - _(Could not lookup controller model for '%s'), def-src); + _(Could not lookup controller model for '%s'), src); goto cleanup; } @@ -427,7 +427,7 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, } else { virReportError(VIR_ERR_INTERNAL_ERROR, _(Found unexpected controller model '%s' for disk '%s'), - vmDiskFileInfo-controllerType, def-src); + vmDiskFileInfo-controllerType, src); goto cleanup; } @@ -3045,6 +3045,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; +const char *src; memset(data, 0, sizeof(data)); @@ -3121,7 +3122,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) for (i = 0; i def-ndisks; ++i) { if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK -def-disks[i]-type == VIR_DOMAIN_DISK_TYPE_FILE) { +virDomainDiskGetType(def-disks[i]) == VIR_DOMAIN_DISK_TYPE_FILE) { disk = def-disks[i]; break; } @@ -3134,22 +3135,23 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } -if (!disk-src) { +src = virDomainDiskGetSource(disk); +if (!src) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(First file-based harddisk has no source, cannot deduce datastore and path for VMX file)); goto cleanup; } -if (esxUtil_ParseDatastorePath(disk-src, datastoreName, directoryName, +if (esxUtil_ParseDatastorePath(src, datastoreName, directoryName, NULL) 0) { goto cleanup; } -if (! virFileHasSuffix(disk-src, .vmdk)) { +if (! virFileHasSuffix(src, .vmdk)) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Expecting source '%s' of first file-based harddisk to - be a VMDK image), disk-src); + be a VMDK image), src); goto cleanup; } -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/18] conf: use disk source accessors in vbox/
Part of a series of cleanups to use new accessor methods. * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc, vboxAttachDrives) (vboxDomainAttachDeviceImpl, vboxDomainDetachDevice): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- src/vbox/vbox_tmpl.c | 137 +-- 1 file changed, 77 insertions(+), 60 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 2aeddd0..9df25c5 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2738,7 +2738,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (VIR_ALLOC(def-disks[i]) = 0) { def-disks[i]-device = VIR_DOMAIN_DISK_DEVICE_DISK; def-disks[i]-bus = VIR_DOMAIN_DISK_BUS_IDE; -def-disks[i]-type = VIR_DOMAIN_DISK_TYPE_FILE; +virDomainDiskSetType(def-disks[i], + VIR_DOMAIN_DISK_TYPE_FILE); } } } @@ -2755,7 +2756,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def-disks[hddNum]-readonly = true; -ignore_value(VIR_STRDUP(def-disks[hddNum]-src, hddlocation)); +ignore_value(virDomainDiskSetSource(def-disks[hddNum], +hddlocation)); ignore_value(VIR_STRDUP(def-disks[hddNum]-dst, hda)); hddNum++; @@ -2776,7 +2778,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def-disks[hddNum]-readonly = true; -ignore_value(VIR_STRDUP(def-disks[hddNum]-src, hddlocation)); +ignore_value(virDomainDiskSetSource(def-disks[hddNum], +hddlocation)); ignore_value(VIR_STRDUP(def-disks[hddNum]-dst, hdb)); hddNum++; @@ -2797,7 +2800,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def-disks[hddNum]-readonly = true; -ignore_value(VIR_STRDUP(def-disks[hddNum]-src, hddlocation)); +ignore_value(virDomainDiskSetSource(def-disks[hddNum], +hddlocation)); ignore_value(VIR_STRDUP(def-disks[hddNum]-dst, hdd)); hddNum++; @@ -2884,10 +2888,11 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { medium-vtbl-GetLocation(medium, mediumLocUtf16); VBOX_UTF16_TO_UTF8(mediumLocUtf16, mediumLocUtf8); VBOX_UTF16_FREE(mediumLocUtf16); -ignore_value(VIR_STRDUP(def-disks[diskCount]-src, mediumLocUtf8)); +ignore_value(virDomainDiskSetSource(def-disks[diskCount], +mediumLocUtf8)); VBOX_UTF8_FREE(mediumLocUtf8); -if (!(def-disks[diskCount]-src)) { +if (!virDomainDiskGetSource(def-disks[diskCount])) { VBOX_RELEASE(medium); VBOX_RELEASE(storageController); error = true; @@ -2936,7 +2941,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (readOnly == PR_TRUE) def-disks[diskCount]-readonly = true; -def-disks[diskCount]-type = VIR_DOMAIN_DISK_TYPE_FILE; +virDomainDiskSetType(def-disks[diskCount], + VIR_DOMAIN_DISK_TYPE_FILE); VBOX_RELEASE(medium); VBOX_RELEASE(storageController); @@ -3211,9 +3217,10 @@ sharedFoldersCleanup: if (VIR_ALLOC(def-disks[def-ndisks - 1]) = 0) { def-disks[def-ndisks - 1]-device = VIR_DOMAIN_DISK_DEVICE_CDROM; def-disks[def-ndisks - 1]-bus = VIR_DOMAIN_DISK_BUS_IDE; -def-disks[def-ndisks - 1]-type = VIR_DOMAIN_DISK_TYPE_FILE; +virDomainDiskSetType(def-disks[def-ndisks - 1], + VIR_DOMAIN_DISK_TYPE_FILE); def-disks[def-ndisks - 1]-readonly = true; -ignore_value(VIR_STRDUP(def-disks[def-ndisks - 1]-src, location)); + ignore_value(virDomainDiskSetSource(def-disks[def-ndisks - 1], location)); ignore_value(VIR_STRDUP(def-disks[def-ndisks - 1]-dst, hdc)); def-ndisks--; } else
Re: [libvirt] [PATCH v2] nwfilter: Fix double free of pointer
On Wed, Mar 19, 2014 at 11:47:09AM -0400, Stefan Berger wrote: From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/18] conf: use disk source accessors in conf/
Part of a series of cleanups to use new accessor methods. Several places in domain_conf.c still open-code raw field access, but that code will be touched later with the diskDef struct split so I'm avoiding churn here. * src/conf/domain_audit.c (virDomainAuditStart): Use accessors. * src/conf/domain_conf.c (virDomainDiskIndexByName) (virDomainDiskPathByName, virDomainDiskDefForeachPath) (virDomainDiskSourceIsBlockType): Likewise. * src/conf/snapshot_conf.c (virDomainSnapshotAlignDisks): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/conf/domain_audit.c | 7 --- src/conf/domain_conf.c | 20 +++- src/conf/snapshot_conf.c | 2 +- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 69632b0..e8bd460 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -799,9 +799,10 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) size_t i; for (i = 0; i vm-def-ndisks; i++) { -virDomainDiskDefPtr disk = vm-def-disks[i]; -if (disk-src) /* Skips CDROM without media initially inserted */ -virDomainAuditDisk(vm, NULL, disk-src, start, true); +const char *src = virDomainDiskGetSource(vm-def-disks[i]); + +if (src) /* Skips CDROM without media initially inserted */ +virDomainAuditDisk(vm, NULL, src, start, true); } for (i = 0; i vm-def-nfss; i++) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d2724ca..a0e07c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10233,8 +10233,7 @@ virDomainDiskIndexByName(virDomainDefPtr def, const char *name, if (*name != '/') { if (STREQ(vdisk-dst, name)) return i; -} else if (vdisk-src - STREQ(vdisk-src, name)) { +} else if (STREQ_NULLABLE(virDomainDiskGetSource(vdisk), name)) { if (allow_ambiguous) return i; if (candidate = 0) @@ -10253,7 +10252,7 @@ virDomainDiskPathByName(virDomainDefPtr def, const char *name) { int idx = virDomainDiskIndexByName(def, name, true); -return idx 0 ? NULL : def-disks[idx]-src; +return idx 0 ? NULL : virDomainDiskGetSource(def-disks[idx]); } int virDomainDiskInsert(virDomainDefPtr def, @@ -18532,14 +18531,16 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, int ret = -1; size_t depth = 0; virStorageFileMetadata *tmp; +const char *path = virDomainDiskGetSource(disk); +int type = virDomainDiskGetType(disk); -if (!disk-src || disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK || -(disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME +if (!path || type == VIR_DOMAIN_DISK_TYPE_NETWORK || +(type == VIR_DOMAIN_DISK_TYPE_VOLUME disk-srcpool disk-srcpool-mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) return 0; -if (iter(disk, disk-src, 0, opaque) 0) +if (iter(disk, path, 0, opaque) 0) goto cleanup; tmp = disk-backingChain; @@ -19409,16 +19410,17 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) /* No reason to think the disk source is block type if * the source is empty */ -if (!def-src) +if (!virDomainDiskGetSource(def)) return false; -if (def-type == VIR_DOMAIN_DISK_TYPE_BLOCK) +if (virDomainDiskGetType(def) == VIR_DOMAIN_DISK_TYPE_BLOCK) return true; /* For volume types, check the srcpool. * If it's a block type source pool, then it's possible */ -if (def-type == VIR_DOMAIN_DISK_TYPE_VOLUME def-srcpool +if (virDomainDiskGetType(def) == VIR_DOMAIN_DISK_TYPE_VOLUME +def-srcpool def-srcpool-voltype == VIR_STORAGE_VOL_BLOCK) { /* We don't think the volume accessed by remote URI is * block type source, since we can't/shouldn't manage it diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 6fa14ed..0509743 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -558,7 +558,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, if (disk-snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL !disk-file) { -const char *original = def-dom-disks[i]-src; +const char *original = virDomainDiskGetSource(def-dom-disks[i]); const char *tmp; struct stat sb; -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/18] conf: use disk source accessors in locking/
Part of a series of cleanups to use new accessor methods. * src/locking/domain_lock.c (virDomainLockManagerAddDisk): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- src/locking/domain_lock.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 643a875..ed37dd9 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -1,7 +1,7 @@ /* * domain_lock.c: Locking for domain lifecycle operations * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -72,12 +72,15 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock, virDomainDiskDefPtr disk) { unsigned int diskFlags = 0; -if (!disk-src) +const char *src = virDomainDiskGetSource(disk); +int type = virDomainDiskGetType(disk); + +if (!src) return 0; -if (!(disk-type == VIR_DOMAIN_DISK_TYPE_BLOCK || - disk-type == VIR_DOMAIN_DISK_TYPE_FILE || - disk-type == VIR_DOMAIN_DISK_TYPE_DIR)) +if (!(type == VIR_DOMAIN_DISK_TYPE_BLOCK || + type == VIR_DOMAIN_DISK_TYPE_FILE || + type == VIR_DOMAIN_DISK_TYPE_DIR)) return 0; if (disk-readonly) @@ -85,14 +88,14 @@ static int virDomainLockManagerAddDisk(virLockManagerPtr lock, if (disk-shared) diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; -VIR_DEBUG(Add disk %s, disk-src); +VIR_DEBUG(Add disk %s, src); if (virLockManagerAddResource(lock, VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, - disk-src, + src, 0, NULL, diskFlags) 0) { -VIR_DEBUG(Failed add disk %s, disk-src); +VIR_DEBUG(Failed add disk %s, src); return -1; } return 0; -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Zombie process after open libvirt connection
On 19.03.2014 12:10, Carlos Rodrigues wrote: Hello Michal, I am using libvirt 1.1.3 and perl-Sys-Virt 1.1.3 and perl-5.16 on Fedora 19 x86_64 The zombie process appears after open libvirt connection with qemu-tls, and perl module is binding for libvirt library XS. Here is my running example with zombie process: $ perl test-chldhandle-bug-fixed.pl sleep 15 echo ps axf | grep perl echo [2] 12427 init... pid=12427 while... fork 1 end... pid=12430 receive chld fork 2 end... pid=12431 receive chld 2014-03-19 11:06:38.712+: 12427: info : libvirt version: 1.1.3.1, package: 2.fc19 (Unknown, 2014-03-17-15:02:00, cmar-laptop.lan) 2014-03-19 11:06:38.712+: 12427: warning : virNetTLSContextCheckCertificate:1140 : Certificate check failed Certificate [session] owner does not match the hostname 10.10.4.249 connection open fork 3 end... pid=12432 fork 4 end... pid=12440 12427 pts/2S 0:00 | \_ perl test-chldhandle-bug-fixed.pl 12432 pts/2Z 0:00 | | \_ [perl] defunct 12440 pts/2Z 0:00 | | \_ [perl] defunct 12442 pts/2S+ 0:00 | \_ grep --color=auto perl Aha! It seems like this is only present if using tls, I was unable to reproduce this with tcp or unix sockets. And when using tcp I can see SIGCHLD being delivered while with tls it is not. That makes me wonder if either libvirt or gnutls silently sets signal mask and not restore it back. Because if I take a look at signal mask I can clearly see SIGCHLD to be blocked (from /proc/$pid/status): SigPnd: ShdPnd: 0001 SigBlk: 08011000 SigIgn: 1080 SigCgt: 00018001 What we can see here is, SigBlk (the bitmask of blocked signals) contains 0x801100 which is SIGPIPE, SIGCHLD and SIGWINCH. Right, why would libvirt care about SIGWINCH anyway? Git greping it leads us to virNetClientSetTLSSession(). I can clearly see there we are adding just those three signals to a mask. Then setting this mask just prior to calling poll() and then restoring back. Oh wait, we are not! pthread_sigmask(SIG_BLOCK,...) is just adding new signals to the mask, not overwriting the old one. So yes, this is clearly libvirt bug. If I use SIG_SETMASK there, I am no longer getting any zombies. I'll post the patch shortly. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/18] conf: use disk source accessors in qemu/
Part of a series of cleanups to use new accessor methods. * src/qemu/qemu_conf.c (qemuCheckSharedDevice) (qemuAddSharedDevice, qemuRemoveSharedDevice, qemuSetUnprivSGIO): Use accessors. * src/qemu/qemu_domain.c (qemuDomainDeviceDefPostParse) (qemuDomainObjCheckDiskTaint, qemuDomainSnapshotForEachQcow2Raw) (qemuDomainCheckRemoveOptionalDisk, qemuDomainCheckDiskPresence) (qemuDiskChainCheckBroken, qemuDomainDetermineDiskChain): Likewise. * src/qemu/qemu_hotplug.c (qemuDomainChangeEjectableMedia) (qemuDomainCheckEjectableMedia) (qemuDomainAttachVirtioDiskDevice, qemuDomainAttachSCSIDisk) (qemuDomainAttachUSBMassstorageDevice) (qemuDomainAttachDeviceDiskLive, qemuDomainRemoveDiskDevice) (qemuDomainDetachVirtioDiskDevice, qemuDomainDetachDiskDevice): Likewise. * src/qemu/qemu_migration.c (qemuMigrationStartNBDServer) (qemuMigrationDriveMirror, qemuMigrationCancelDriveMirror) (qemuMigrationIsSafe): Likewise. * src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase) (qemuProcessHandleIOError, qemuProcessHandleBlockJob) (qemuProcessInitPasswords): Likewise. * src/qemu/qemu_driver.c (qemuDomainChangeDiskMediaLive) (qemuDomainGetBlockInfo, qemuDiskPathToAlias): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_conf.c | 22 + src/qemu/qemu_domain.c| 60 +--- src/qemu/qemu_driver.c| 18 +++ src/qemu/qemu_hotplug.c | 116 ++ src/qemu/qemu_migration.c | 17 +++ src/qemu/qemu_process.c | 11 +++-- 6 files changed, 135 insertions(+), 109 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 91c5e3a..d280c2c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1,7 +1,7 @@ /* * qemu_conf.c: QEMU configuration management * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -744,6 +744,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, char *key = NULL; int val; int ret = 0; +const char *src; if (dev-type == VIR_DOMAIN_DEVICE_DISK) { disk = dev-data.disk; @@ -757,7 +758,8 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, return 0; } -if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk-src, NULL))) { +src = virDomainDiskGetSource(disk); +if (!(sysfs_path = virGetUnprivSGIOSysfsPath(src, NULL))) { ret = -1; goto cleanup; } @@ -768,7 +770,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!virFileExists(sysfs_path)) goto cleanup; -if (!(key = qemuGetSharedDeviceKey(disk-src))) { +if (!(key = qemuGetSharedDeviceKey(src))) { ret = -1; goto cleanup; } @@ -779,7 +781,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!(virHashLookup(sharedDevices, key))) goto cleanup; -if (virGetDeviceUnprivSGIO(disk-src, NULL, val) 0) { +if (virGetDeviceUnprivSGIO(src, NULL, val) 0) { ret = -1; goto cleanup; } @@ -791,7 +793,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, disk-sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED)) goto cleanup; -if (disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME) { +if (virDomainDiskGetType(disk) == VIR_DOMAIN_DISK_TYPE_VOLUME) { virReportError(VIR_ERR_OPERATION_INVALID, _(sgio of shared disk 'pool=%s' 'volume=%s' conflicts with other active domains), @@ -800,7 +802,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, } else { virReportError(VIR_ERR_OPERATION_INVALID, _(sgio of shared disk '%s' conflicts with other - active domains), disk-src); + active domains), src); } ret = -1; @@ -917,7 +919,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, goto cleanup; if (dev-type == VIR_DOMAIN_DEVICE_DISK) { -if (!(key = qemuGetSharedDeviceKey(disk-src))) +if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk goto cleanup; } else { if (!(dev_name = virSCSIDeviceGetDevName(NULL, @@ -1022,7 +1024,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, qemuDriverLock(driver); if (dev-type == VIR_DOMAIN_DEVICE_DISK) { -if (!(key = qemuGetSharedDeviceKey(disk-src))) +if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk goto cleanup; } else { if (!(dev_name = virSCSIDeviceGetDevName(NULL, @@ -1079,7 +1081,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) virDomainDiskDefPtr disk = NULL; virDomainHostdevDefPtr hostdev = NULL; char *sysfs_path = NULL; -char *path = NULL; +const char *path = NULL; int val = -1; int ret = 0; @@ -1093,7 +1095,7 @@
[libvirt] [PATCH 17/18] conf: use disk source accessors in xenxs/
Part of a series of cleanups to use new accessor methods. * src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenParseSxpr) (xenFormatSxprDisk, xenFormatSxpr): Use accessors. * src/xenxs/xen_xm.c (xenParseXM, xenFormatXMDisk, xenFormatXM): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- This one is a bit trickier to review, in that it is replacing in-place strndup over to calls to a function that mallocs a copy of the input string. While it compiles and passes 'make check', I don't actually have a xen setup to actually prove that it works. src/xenxs/xen_sxpr.c | 111 ++ src/xenxs/xen_xm.c | 113 +++ 2 files changed, 118 insertions(+), 106 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 01d1ca1..e03e254 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1,7 +1,7 @@ /* * xen_sxpr.c: Xen SEXPR parsing functions * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2011 Univention GmbH * Copyright (C) 2005 Anthony Liguori aligu...@us.ibm.com * @@ -401,24 +401,23 @@ xenParseSxprDisks(virDomainDefPtr def, if (sexpr_lookup(node, device/tap2) STRPREFIX(src, tap:)) { -if (VIR_STRDUP(disk-driverName, tap2) 0) +if (virDomainDiskSetDriver(disk, tap2) 0) goto error; } else { -if (VIR_ALLOC_N(disk-driverName, (offset-src)+1) 0) +char *tmp; +if (VIR_STRNDUP(tmp, src, offset - src) 0) goto error; -if (virStrncpy(disk-driverName, src, offset-src, - (offset-src)+1) == NULL) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Driver name %s too big for destination), - src); +if (virDomainDiskSetDriver(disk, tmp) 0) { +VIR_FREE(tmp); goto error; } +VIR_FREE(tmp); } src = offset + 1; -if (STREQ(disk-driverName, tap) || -STREQ(disk-driverName, tap2)) { +if (STREQ(virDomainDiskGetDriver(disk), tap) || +STREQ(virDomainDiskGetDriver(disk), tap2)) { char *driverType = NULL; offset = strchr(src, ':'); @@ -431,12 +430,12 @@ xenParseSxprDisks(virDomainDefPtr def, if (VIR_STRNDUP(driverType, src, offset - src) 0) goto error; if (STREQ(driverType, aio)) -disk-format = VIR_STORAGE_FILE_RAW; +virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); else -disk-format = -virStorageFileFormatTypeFromString(driverType); +virDomainDiskSetFormat(disk, + virStorageFileFormatTypeFromString(driverType)); VIR_FREE(driverType); -if (disk-format = 0) { +if (virDomainDiskGetFormat(disk) = 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown driver type %s), src); goto error; @@ -448,17 +447,17 @@ xenParseSxprDisks(virDomainDefPtr def, so we assume common case here. If blktap becomes omnipotent, we can revisit this, perhaps stat()'ing the src file in question */ -disk-type = VIR_DOMAIN_DISK_TYPE_FILE; -} else if (STREQ(disk-driverName, phy)) { -disk-type = VIR_DOMAIN_DISK_TYPE_BLOCK; -} else if (STREQ(disk-driverName, file)) { -disk-type = VIR_DOMAIN_DISK_TYPE_FILE; +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); +} else if (STREQ(virDomainDiskGetDriver(disk), phy)) { +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); +} else if (STREQ(virDomainDiskGetDriver(disk), file)) { +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); } } else { /* No CDROM media so can't really tell. We'll just call if a FILE for now and update when media is inserted later */ -disk-type = VIR_DOMAIN_DISK_TYPE_FILE; +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); } if (STREQLEN(dst, ioemu:, 6)) @@ -482,7 +481,7 @@
[libvirt] [PATCH 07/18] conf: use disk source accessors in lxc/
Part of a series of cleanups to use new accessor methods. * src/lxc/lxc_cgroup.c (virLXCCgroupSetupDeviceACL): Use accessors. * src/lxc/lxc_controller.c (virLXCControllerSetupLoopDeviceDisk) (virLXCControllerSetupNBDDeviceDisk) (virLXCControllerSetupLoopDevices, virLXCControllerSetupDisk): Likewise. * src/lxc/lxc_driver.c (lxcDomainAttachDeviceDiskLive) (lxcDomainDetachDeviceDiskLive): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/lxc/lxc_cgroup.c | 6 ++--- src/lxc/lxc_controller.c | 69 +++- src/lxc/lxc_driver.c | 27 +++ 3 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5a1718d..da5ccf5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_cgroup.c: LXC cgroup helpers @@ -372,11 +372,11 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, VIR_DEBUG(Allowing any disk block devs); for (i = 0; i def-ndisks; i++) { -if (def-disks[i]-type != VIR_DOMAIN_DISK_TYPE_BLOCK) +if (!virDomainDiskSourceIsBlockType(def-disks[i])) continue; if (virCgroupAllowDevicePath(cgroup, - def-disks[i]-src, + virDomainDiskGetSource(def-disks[i]), (def-disks[i]-readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW) | diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 874e196..6ed13fb 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_controller.c: linux container process controller @@ -382,21 +382,24 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) { int lofd; char *loname = NULL; +const char *src = virDomainDiskGetSource(disk); -if ((lofd = virFileLoopDeviceAssociate(disk-src, loname)) 0) +if ((lofd = virFileLoopDeviceAssociate(src, loname)) 0) return -1; VIR_DEBUG(Changing disk %s to use type=block for dev %s, - disk-src, loname); + src, loname); /* * We now change it into a block device type, so that * the rest of container setup 'just works' */ -disk-type = VIR_DOMAIN_DISK_TYPE_BLOCK; -VIR_FREE(disk-src); -disk-src = loname; -loname = NULL; +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); +if (virDomainDiskSetSource(disk, loname) 0) { +VIR_FREE(loname); +return -1; +} +VIR_FREE(loname); return lofd; } @@ -435,28 +438,33 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) { char *dev; +const char *src = virDomainDiskGetSource(disk); +int format = virDomainDiskGetFormat(disk); -if (disk-format = VIR_STORAGE_FILE_NONE) { +if (format = VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(An explicit disk format must be specified)); return -1; } -if (virFileNBDDeviceAssociate(disk-src, - disk-format, +if (virFileNBDDeviceAssociate(src, + format, disk-readonly, dev) 0) return -1; VIR_DEBUG(Changing disk %s to use type=block for dev %s, - disk-src, dev); + src, dev); /* * We now change it into a block device type, so that * the rest of container setup 'just works' */ -disk-type = VIR_DOMAIN_DISK_TYPE_BLOCK; -VIR_FREE(disk-src); -disk-src = dev; +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); +if (virDomainDiskSetSource(disk, dev) 0) { +VIR_FREE(dev); +return -1; +} +VIR_FREE(dev); return 0; } @@ -519,23 +527,25 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) for (i = 0; i ctrl-def-ndisks; i++) { virDomainDiskDefPtr disk = ctrl-def-disks[i]; int fd; +const char *driver = virDomainDiskGetDriver(disk); +int format = virDomainDiskGetFormat(disk); -if (disk-type != VIR_DOMAIN_DISK_TYPE_FILE) +if (virDomainDiskGetType(disk) != VIR_DOMAIN_DISK_TYPE_FILE) continue; /* If no driverName is set, we prefer 'loop' for * dealing with raw or undefined formats, otherwise * we use 'nbd'. */ -if (STREQ_NULLABLE(disk-driverName, loop) || -
[libvirt] [PATCH 09/18] conf: use disk source accessors in phyp/
Part of a series of cleanups to use new accessor methods. * src/phyp/phyp_driver.c (phypDomainAttachDevice, phypBuildLpar): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- src/phyp/phyp_driver.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index d97b39f..47a317e 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2009 * * phyp_driver.c: ssh layer to access Power Hypervisors @@ -1709,7 +1709,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) managed_system, vios_id); virBufferAsprintf(buf, mkvdev -vdev %s -vadapter %s, - dev-data.disk-src, scsi_adapter); + virDomainDiskGetSource(dev-data.disk), scsi_adapter); if (system_type == HMC) virBufferAddChar(buf, '\''); @@ -1730,7 +1730,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) virBufferAsprintf(buf, -m %s, managed_system); virBufferAsprintf(buf, slot_num,backing_device|grep %s|cut -d, -f1, - dev-data.disk-src); + virDomainDiskGetSource(dev-data.disk)); if (phypExecInt(session, buf, conn, slot) 0) goto cleanup; @@ -3488,7 +3488,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) goto cleanup; } -if (!def-disks[0]-src) { +if (!virDomainDiskGetSource(def-disks[0])) { virReportError(VIR_ERR_XML_ERROR, %s, _(Field src under disk on the domain XML file is missing.)); @@ -3502,7 +3502,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) max_mem=%lld,desired_procs=%d,virtual_scsi_adapters=%s, def-name, def-mem.cur_balloon, def-mem.cur_balloon, def-mem.max_balloon, - (int) def-vcpus, def-disks[0]-src); + (int) def-vcpus, virDomainDiskGetSource(def-disks[0])); ret = phypExecBuffer(session, buf, exit_status, conn, false); if (exit_status 0) { -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 15/18] conf: use disk source accessors in vmx/
Part of a series of cleanups to use new accessor methods. * src/vmx/vmx.c (virVMXHandleLegacySCSIDiskDriverName) (virVMXParseDisk, virVMXFormatDisk, virVMXFormatFloppy): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- src/vmx/vmx.c | 113 ++ 1 file changed, 66 insertions(+), 47 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 1ebb0f9..341d1af 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1,7 +1,7 @@ /* * vmx.c: VMware VMX parsing/formatting functions * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2009-2010 Matthias Bolte matthias.bo...@googlemail.com * * This library is free software; you can redistribute it and/or @@ -1060,22 +1060,27 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def, int model; size_t i; virDomainControllerDefPtr controller = NULL; +const char *driver = virDomainDiskGetDriver(disk); +char *copy; -if (disk-bus != VIR_DOMAIN_DISK_BUS_SCSI || disk-driverName == NULL) { +if (disk-bus != VIR_DOMAIN_DISK_BUS_SCSI || !driver) { return 0; } -tmp = disk-driverName; +if (VIR_STRDUP(copy, driver) 0) +return -1; +tmp = copy; for (; *tmp != '\0'; ++tmp) { *tmp = c_tolower(*tmp); } -model = virDomainControllerModelSCSITypeFromString(disk-driverName); +model = virDomainControllerModelSCSITypeFromString(copy); +VIR_FREE(copy); if (model 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _(Unknown driver name '%s'), disk-driverName); + _(Unknown driver name '%s'), driver); return -1; } @@ -1098,7 +1103,7 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def, } else if (controller-model != model) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Inconsistent SCSI controller model ('%s' is not '%s') - for SCSI controller index %d), disk-driverName, + for SCSI controller index %d), driver, virDomainControllerModelSCSITypeToString(controller-model), controller-idx); return -1; @@ -2179,15 +2184,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } } -(*def)-type = VIR_DOMAIN_DISK_TYPE_FILE; -(*def)-src = ctx-parseFileName(fileName, ctx-opaque); +virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE); +if (virDomainDiskSetSource(*def, + ctx-parseFileName(fileName, + ctx-opaque)) 0) +goto cleanup; (*def)-cachemode = writeThrough ? VIR_DOMAIN_DISK_CACHE_WRITETHRU : VIR_DOMAIN_DISK_CACHE_DEFAULT; if (mode) (*def)-transient = STRCASEEQ(mode, independent-nonpersistent); -if ((*def)-src == NULL) { +if (!virDomainDiskGetSource(*def)) { goto cleanup; } } else if (virFileHasSuffix(fileName, .iso) || @@ -2219,10 +2227,13 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } } -(*def)-type = VIR_DOMAIN_DISK_TYPE_FILE; -(*def)-src = ctx-parseFileName(fileName, ctx-opaque); +virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE); +if (virDomainDiskSetSource(*def, + ctx-parseFileName(fileName, + ctx-opaque)) 0) +goto cleanup; -if ((*def)-src == NULL) { +if (!virDomainDiskGetSource(*def)) { goto cleanup; } } else if (virFileHasSuffix(fileName, .vmdk)) { @@ -2234,26 +2245,24 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con */ goto ignore; } else if (STRCASEEQ(deviceType, atapi-cdrom)) { -(*def)-type = VIR_DOMAIN_DISK_TYPE_BLOCK; +virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_BLOCK); if (STRCASEEQ(fileName, auto detect)) { -(*def)-src = NULL; +ignore_value(virDomainDiskSetSource(*def, NULL)); (*def)-startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL; -} else { -(*def)-src = fileName; -fileName = NULL; +} else if (virDomainDiskSetSource(*def, fileName) 0) { +goto cleanup; } } else if (STRCASEEQ(deviceType, cdrom-raw)) { /* Raw access CD-ROMs actually are device='lun' */
Re: [libvirt] [PATCH] network: fix problems with SRV
On Wed, Mar 19, 2014 at 10:17:20AM -0600, Laine Stump wrote: On 03/18/2014 01:09 AM, Martin Kletzander wrote: On Tue, Mar 18, 2014 at 12:07:17AM -0600, Laine Stump wrote: A patch submitted by Steven Malin last week pointed out a problem with libvirt's DNS SRV record configuration: https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html When searching for that message later, I found another series that had been posted by Guannan Ren back in 2012 that somehow slipped between the cracks: https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html That patch was very much out of date, but also pointed out some real problems. This patch fixes all the noted problems by refactoring virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then verifies those fixes by added several new records to the test case. Problems fixed: * both service and protocol now have an underscore (_) prepended on the commandline, as required by RFC2782. srv service='sip' protocol='udp' domain='example.com' target='tests.example.com' port='5060' priority='10' weight='150'/ before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150 after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150 * if domain wasn't specified in the srv element, the extra trailing . will no longer be added to the dnsmasq commandline. srv service='sip' protocol='udp' target='tests.example.com' port='5060' priority='10' weight='150'/ before: srv-host=sip.udp.,tests.example.com,5060,10,150 after: srv-host=_sip._udp,tests.example.com,5060,10,150 * when optional attributes aren't specified, the separating comma is also now not placed on the dnsmasq commandline. If optional attributes in the middle of the line are not specified, they are replaced with 0 in the commandline. srv service='sip' protocol='udp' target='tests.example.com' port='5060'/ before: srv-host=sip.udp.,tests.example.com,5060,, after: srv-host=_sip._udp,tests.example.com,5060 (actually this would have generated an error, because optional attributes weren't really optional). * As a safeguard, the parser checks for a leading _ on service and protocol, and fails if it is found. (Note that we can be guaranteed that no existing valid configuration will have this, since until now the parser had only allowed tcp or udp for protocol, and the bridge driver wasn't prepending _, making it a 100% certainty that there was no example of working SRV record use in the wild). That's valid for protocol, but not for service. For service there was a check for length only and it was not escaped at all. Even though there couldn't be any abuse for that, settings that resulted in generating invalid config file for dnsmasq were parsed and saved without any error from libvirt. Just to make sure what I wrote made sense: my point was that, due to the fact that the current code would only be able to generate records like this: [some-name].tcp.[some-domain] [some-name].udp.[some-domain] (i.e. the protocol could only be the exact string tcp or udp), and since the RFC *requires* that the protocol be prefixed with an underscore, it is impossible that anyone has an actual working config using the existing code (although it may have been saved with no error, it wouldn't do anything useful). Because it is impossible that it could have worked before, we don't need to worry about backward compatibility problems - nothing we do could break a working config. But I *think* what you're pointing out is that, although the config may not be working, it's possible that someone could have made a config like this: srv service='_sip' protocol='udp' domain='example.com' target='sip.example.com' port='5060'/ which they could have saved, even though it would do nothing useful, it would be saved. Then when that system was updated to the new libvirt, that network's config would generate an error while loading, so the network would be ignored. Gah!! Yes, exactly :-( So I *can't* add this new restriction in the parser. I could check for it during networkValidate() in the bridge driver, though. Do you think this is worthwhile? (I think I'll do it as a separate patch in any case). The networkValidate() is called from DefineXML so that would make it ignored as well. I'd suggest networkStartNetwork() after the networkRunHook() since it can add anything. - -/* Check whether protocol value is the supported one */ -if (def-protocol STRNEQ(def-protocol, tcp) -(STRNEQ(def-protocol, udp))) { +if (def-protocol def-protocol[0] == '_') { virReportError(VIR_ERR_XML_DETAIL, I'm not sure we want to allow anything not starting with '_'. OTOH, we allowed anything in the service (even dots and commas and so on
[libvirt] [PATCH] virNetClientSetTLSSession: Restore original signal mask
Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Notes: This is a very old bug. It's worth backporting onto all maint branches we have. src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b288b74..255997b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -792,7 +792,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret 0 (errno == EAGAIN || errno == EINTR)) goto repoll; -ignore_value(pthread_sigmask(SIG_BLOCK, oldmask, NULL)); +ignore_value(pthread_sigmask(SIG_SETMASK, oldmask, NULL)); } ret = virNetTLSContextCheckCertificate(tls, client-tls); @@ -816,7 +816,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret 0 (errno == EAGAIN || errno == EINTR)) goto repoll2; -ignore_value(pthread_sigmask(SIG_BLOCK, oldmask, NULL)); +ignore_value(pthread_sigmask(SIG_SETMASK, oldmask, NULL)); len = virNetTLSSessionRead(client-tls, buf, 1); if (len 0 errno != ENOMSG) { -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/18] conf: use disk source accessors in parallels/
Part of a series of cleanups to use new accessor methods. * src/parallels/parallels_driver.c (parallelsGetHddInfo) (parallelsAddHdd, parallelsApplyDisksParams, parallelsCreateVm): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/parallels/parallels_driver.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 626a1e2..0c985dd 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -2,6 +2,7 @@ * parallels_driver.c: core driver functions for managing * Parallels Cloud Server hosts * + * Copyright (C) 2014 Red Hat, Inc. * Copyright (C) 2012 Parallels, Inc. * * This library is free software; you can redistribute it and/or @@ -301,24 +302,24 @@ parallelsGetHddInfo(virDomainDefPtr def, disk-device = VIR_DOMAIN_DISK_DEVICE_DISK; if (virJSONValueObjectHasKey(value, real) == 1) { -disk-type = VIR_DOMAIN_DISK_TYPE_BLOCK; +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK); if (!(tmp = virJSONValueObjectGetString(value, real))) { parallelsParseError(); return -1; } -if (VIR_STRDUP(disk-src, tmp) 0) +if (virDomainDiskSetSource(disk, tmp) 0) return -1; } else { -disk-type = VIR_DOMAIN_DISK_TYPE_FILE; +virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE); if (!(tmp = virJSONValueObjectGetString(value, image))) { parallelsParseError(); return -1; } -if (VIR_STRDUP(disk-src, tmp) 0) +if (virDomainDiskSetSource(disk, tmp) 0) return -1; } @@ -1647,10 +1648,11 @@ static int parallelsAddHdd(virConnectPtr conn, virStoragePoolObjPtr pool = NULL; virStorageVolPtr vol = NULL; int ret = -1; +const char *src = virDomainDiskGetSource(disk); -if (!(vol = parallelsStorageVolLookupByPathLocked(conn, disk-src))) { +if (!(vol = parallelsStorageVolLookupByPathLocked(conn, src))) { virReportError(VIR_ERR_INVALID_ARG, - _(Can't find volume with path '%s'), disk-src); + _(Can't find volume with path '%s'), src); return -1; } @@ -1662,11 +1664,11 @@ static int parallelsAddHdd(virConnectPtr conn, goto cleanup; } -voldef = virStorageVolDefFindByPath(pool, disk-src); +voldef = virStorageVolDefFindByPath(pool, src); if (!voldef) { virReportError(VIR_ERR_INVALID_ARG, _(Can't find storage volume definition for path '%s'), - disk-src); + src); goto cleanup; } @@ -1725,7 +1727,8 @@ parallelsApplyDisksParams(virConnectPtr conn, parallelsDomObjPtr pdom, if (olddisk-bus != newdisk-bus || olddisk-info.addr.drive.target != newdisk-info.addr.drive.target || -!STREQ_NULLABLE(olddisk-src, newdisk-src)) { +!STREQ_NULLABLE(virDomainDiskGetSource(olddisk), +virDomainDiskGetSource(newdisk))) { char prlname[16]; char strpos[16]; @@ -2201,16 +2204,18 @@ parallelsCreateVm(virConnectPtr conn, virDomainDefPtr def) virStoragePoolObjPtr pool = NULL; virStorageVolPtr vol = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *src; for (i = 0; i def-ndisks; i++) { if (def-disks[i]-device != VIR_DOMAIN_DISK_DEVICE_DISK) continue; -vol = parallelsStorageVolLookupByPathLocked(conn, def-disks[i]-src); +src = virDomainDiskGetSource(def-disks[i]); +vol = parallelsStorageVolLookupByPathLocked(conn, src); if (!vol) { virReportError(VIR_ERR_INVALID_ARG, _(Can't find volume with path '%s'), - def-disks[i]-src); + src); return -1; } break; @@ -2234,11 +2239,11 @@ parallelsCreateVm(virConnectPtr conn, virDomainDefPtr def) goto error; } -privvol = virStorageVolDefFindByPath(pool, def-disks[i]-src); +privvol = virStorageVolDefFindByPath(pool, src); if (!privvol) { virReportError(VIR_ERR_INVALID_ARG, _(Can't find storage volume definition for path '%s'), - def-disks[i]-src); + src); goto error2; } -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 16/18] conf: use disk source accessors in xen/
Part of a series of cleanups to use new accessor methods. * src/xen/xend_internal.c (virDomainXMLDevID): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- src/xen/xend_internal.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 4b10f42..55604bc 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1,7 +1,7 @@ /* * xend_internal.c: access to Xen though the Xen Daemon interface * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori aligu...@us.ibm.com * * This library is free software; you can redistribute it and/or @@ -3338,14 +3338,11 @@ virDomainXMLDevID(virConnectPtr conn, xenUnifiedPrivatePtr priv = conn-privateData; char *xref; char *tmp; +const char *driver = virDomainDiskGetDriver(dev-data.disk); if (dev-type == VIR_DOMAIN_DEVICE_DISK) { -if (dev-data.disk-driverName -STREQ(dev-data.disk-driverName, tap)) -strcpy(class, tap); -else if (dev-data.disk-driverName -STREQ(dev-data.disk-driverName, tap2)) -strcpy(class, tap2); +if (STREQ_NULLABLE(driver, tap) || STREQ_NULLABLE(driver, tap2)) +strcpy(class, driver); else strcpy(class, vbd); -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix virQEMUCapsLoadCache leaks
On Wed, Mar 19, 2014 at 05:17:40PM +0100, Ján Tomko wrote: Valgrind reported leaking of maxCpus and arch strings from virXPathString, as well as the leak of the machineMaxCpus array. Use 'tmp' for the strings we don't want to free, to allow freeing of 'str' in the cleanup label and free machineMaxCpus in virCapsReset too. --- src/qemu/qemu_capabilities.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2914200..e742c03 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2376,7 +2376,8 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, int n; xmlNodePtr *nodes = NULL; xmlXPathContextPtr ctxt = NULL; -char *str; +char *str = NULL; +char *tmp; long long int l; if (!(doc = virXMLParseFile(filename))) @@ -2432,7 +2433,6 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, if (flag 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown qemu capabilities flag %s), str); -VIR_FREE(str); goto cleanup; } VIR_FREE(str); @@ -2463,6 +2463,7 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, _(unknown arch %s in QEMU capabilities cache), str); goto cleanup; } +VIR_FREE(str); if ((n = virXPathNodeSet(./cpu, ctxt, nodes)) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -2476,12 +2477,12 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, goto cleanup; for (i = 0; i n; i++) { -if (!(str = virXMLPropString(nodes[i], name))) { +if (!(tmp = virXMLPropString(nodes[i], name))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(missing cpu name in QEMU capabilities cache)); goto cleanup; } -qemuCaps-cpuDefinitions[i] = str; +qemuCaps-cpuDefinitions[i] = tmp; Wouldn't it be easier to throw away tmp and do: qemuCaps-cpuDefinitions[i] = virXMLPropString(nodes[i], name); if (!qemuCaps-cpuDefinitions[i]) as with aliases etc.? } } VIR_FREE(nodes); @@ -2503,12 +2504,12 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, goto cleanup; for (i = 0; i n; i++) { -if (!(str = virXMLPropString(nodes[i], name))) { +if (!(tmp = virXMLPropString(nodes[i], name))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(missing machine name in QEMU capabilities cache)); goto cleanup; } -qemuCaps-machineTypes[i] = str; +qemuCaps-machineTypes[i] = tmp; Same here? qemuCaps-machineAliases[i] = virXMLPropString(nodes[i], alias); @@ -2519,12 +2520,14 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, _(malformed machine cpu count in QEMU capabilities cache)); goto cleanup; } +VIR_FREE(str); } } VIR_FREE(nodes); ret = 0; - cleanup: +cleanup: I wish someone would add a HACKING rule about required spaces before labels and enforce them in syntax-check. This makes git diffs a bit less usable :( ACK with the suggestions being optional to squash in. Martin +VIR_FREE(str); VIR_FREE(nodes); xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); @@ -2668,6 +2671,7 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps) } VIR_FREE(qemuCaps-machineTypes); VIR_FREE(qemuCaps-machineAliases); +VIR_FREE(qemuCaps-machineMaxCpus); qemuCaps-nmachineTypes = 0; } -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 13/13] libxl: add migration support
Michal Privoznik wrote: On 13.03.2014 23:11, Jim Fehlig wrote: This patch adds initial migration support to the libxl driver, using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration functions. Signed-off-by: Jim Fehlig jfeh...@suse.com --- V2: - Now that the libxl driver supports hostdev passthrough, ensure domain has no hostdevs before migration - Fail early in libxlDomainMigrateBegin3Params if domain is inactive - Change name of local variable in libxlDoMigrateSend from 'live' to 'xl_flags'. po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/libxl/libxl_conf.h | 6 + src/libxl/libxl_domain.h| 1 + src/libxl/libxl_driver.c| 225 + src/libxl/libxl_migration.c | 598 src/libxl/libxl_migration.h | 78 ++ 7 files changed, 911 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3306dc1..6fc5266 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4313,6 +4323,216 @@ cleanup: return ret; } +static char * +libxlDomainMigrateBegin3Params(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ +const char *xmlin = NULL; +const char *dname = NULL; +virDomainObjPtr vm = NULL; + +virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL); +if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) 0) +return NULL; + +if (virTypedParamsGetString(params, nparams, +VIR_MIGRATE_PARAM_DEST_XML, +xmlin) 0 || +virTypedParamsGetString(params, nparams, +VIR_MIGRATE_PARAM_DEST_NAME, +dname) 0) I'd expect @dname to be used somewhere... +return NULL; + +if (!(vm = libxlDomObjFromDomain(domain))) +return NULL; + +if (virDomainMigrateBegin3ParamsEnsureACL(domain-conn, vm-def) 0) { +virObjectUnlock(vm); +return NULL; +} + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +virObjectUnlock(vm); +return NULL; +} + +return libxlDomainMigrationBegin(domain-conn, vm, xmlin); +} + diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c new file mode 100644 index 000..01023db --- /dev/null +++ b/src/libxl/libxl_migration.c @@ -0,0 +1,598 @@ +/* + * libxl_migration.c: methods for handling migration with libxenlight + * + * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Authors: + * Jim Fehlig jfeh...@suse.com + * Chunyan Liu cy...@suse.com + */ + +#include config.h + +#include internal.h +#include virlog.h +#include virerror.h +#include virconf.h +#include datatypes.h +#include virfile.h +#include viralloc.h +#include viruuid.h +#include vircommand.h +#include virstring.h +#include rpc/virnetsocket.h +#include libxl_domain.h +#include libxl_driver.h +#include libxl_conf.h +#include libxl_migration.h + +#define VIR_FROM_THIS VIR_FROM_LIBXL + +typedef struct _libxlMigrateReceiveArgs { +virConnectPtr conn; +virDomainObjPtr vm; + +/* for freeing listen sockets */ +virNetSocketPtr *socks; +size_t nsocks; +} libxlMigrateReceiveArgs; + +static const char libxlMigrateReceiverReady[] = +libvirt libxl migration receiver ready, send binary domain data; +static const char libxlMigrateReceiverFinish[] = +domain received, ready to unpause; So you're using these to sync source and destination on migration. Kudos for using text protocol not a binary blob. I haven't followed v1 closely, so what was resolution on this? I mean, my concern is if this is extensible. I was hoping to keep the libxl side of the protocol simple with only a ready to receive and received
Re: [libvirt] [PATCH] virNetClientSetTLSSession: Restore original signal mask
On Wed, Mar 19, 2014 at 06:29:01PM +0100, Michal Privoznik wrote: Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. And indeed we do that on all other places. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- ACK. Notes: This is a very old bug. It's worth backporting onto all maint branches we have. Go ahead (but only the active ones ;-) ) Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: Fix double free of pointer
Daniel P. Berrange berra...@redhat.com wrote on 03/19/2014 01:24:52 PM: On Wed, Mar 19, 2014 at 11:47:09AM -0400, Stefan Berger wrote: From: Stefan Berger stef...@linux.vnet.ibm.com https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) ACK Pushed -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetClientSetTLSSession: Restore original signal mask
On 03/19/2014 11:29 AM, Michal Privoznik wrote: Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Notes: This is a very old bug. It's worth backporting onto all maint branches we have. src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK. And yes, it needs to be backported. Bug was introduced in virnetclient.c at its creation in commit 434de30 (v0.9.3); that code was generalized from src/remote/remote_driver.c, but at that time, remote_driver.c was doing things correctly. Not sure how I missed the corruption (obviously, the refactoring to add the rpc client code wasn't quite a copy and paste, for that typo to have snuck in). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetClientSetTLSSession: Restore original signal mask
On Wed, Mar 19, 2014 at 06:29:01PM +0100, Michal Privoznik wrote: Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Notes: This is a very old bug. It's worth backporting onto all maint branches we have. src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b288b74..255997b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -792,7 +792,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret 0 (errno == EAGAIN || errno == EINTR)) goto repoll; -ignore_value(pthread_sigmask(SIG_BLOCK, oldmask, NULL)); +ignore_value(pthread_sigmask(SIG_SETMASK, oldmask, NULL)); } ret = virNetTLSContextCheckCertificate(tls, client-tls); @@ -816,7 +816,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret 0 (errno == EAGAIN || errno == EINTR)) goto repoll2; -ignore_value(pthread_sigmask(SIG_BLOCK, oldmask, NULL)); +ignore_value(pthread_sigmask(SIG_SETMASK, oldmask, NULL)); len = virNetTLSSessionRead(client-tls, buf, 1); if (len 0 errno != ENOMSG) { ACK, I see other places in our code get this right already Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] network: fix problems with SRV records
A patch submitted by Steven Malin last week pointed out a problem with libvirt's DNS SRV record configuration: https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html When searching for that message later, I found another series that had been posted by Guannan Ren back in 2012 that somehow slipped between the cracks: https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html That patch was very much out of date, but also pointed out some real problems. This patch fixes all the noted problems by refactoring virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then verifies those fixes by added several new records to the test case. Problems fixed: * both service and protocol now have an underscore (_) prepended on the commandline, as required by RFC2782. srv service='sip' protocol='udp' domain='example.com' target='tests.example.com' port='5060' priority='10' weight='150'/ before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150 after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150 * if domain wasn't specified in the srv element, the extra trailing . will no longer be added to the dnsmasq commandline. srv service='sip' protocol='udp' target='tests.example.com' port='5060' priority='10' weight='150'/ before: srv-host=sip.udp.,tests.example.com,5060,10,150 after: srv-host=_sip._udp,tests.example.com,5060,10,150 * when optional attributes aren't specified, the separating comma is also now not placed on the dnsmasq commandline. If optional attributes in the middle of the line are not specified, they are replaced with a default value in the commandline (1 for port, 0 for priority and weight). srv service='sip' protocol='udp' target='tests.example.com' port='5060'/ before: srv-host=sip.udp.,tests.example.com,5060,, after: srv-host=_sip._udp,tests.example.com,5060 (actually the would have generated an error, because optional attributes weren't really optional.) * The allowed characters for both service and protocol are now limited to alphanumerics, plus a few special characters that are found in existing names in /etc/services and /etc/protocols. (One exception is that both of these files contain names with an embedded ., but . can't be used in these fields of an SRV record because it is used as a field separator and there is no method to escape a . into a field.) (Previously only the strings tcp and udp were allowed for protocol, but this restriction has been removed, since RFC2782 specifically says that it isn't limited to those, and that anyway it is case insensitive.) * the domain attribute is no longer required in order to recognize the port, priority, and weight attributes during parsing. Only target is required for this. * if target isn't specified, port, priority, and weight are not allowed (since they are meaningless - an empty target means this service is *not available* for this domain). * port, priority, and weight are now truly optional, as the comments originally suggested, but which was not actually true. --- Changes from V1: https://www.redhat.com/archives/libvir-list/2014-March/msg01172.html src/conf/network_conf.c| 129 ++--- src/network/bridge_driver.c| 80 - .../nat-network-dns-srv-record-minimal.conf| 2 +- .../nat-network-dns-srv-record.conf| 8 +- .../nat-network-dns-srv-record.xml | 8 +- 5 files changed, 149 insertions(+), 78 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 9be06d3..f1e6243 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -924,6 +924,21 @@ error: return -1; } +/* This includes all characters used in the names of current + * /etc/services and /etc/protocols files (on Fedora 20), except ., + * which we can't allow because it would conflict with the use of . + * as a field separator in the SRV record, there appears to be no way + * to escape it in, and the protocols/services that use . in the + * name are obscure and unlikely to be used anyway. + */ +#define PROTOCOL_CHARS \ +abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 \ +-+/ + +#define SERVICE_CHARS \ +abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 \ +_-+/* + static int virNetworkDNSSrvDefParseXML(const char *networkName, xmlNodePtr node, @@ -931,80 +946,108 @@ virNetworkDNSSrvDefParseXML(const char *networkName, virNetworkDNSSrvDefPtr def, bool partialOkay) { +int ret; +xmlNodePtr save_ctxt = ctxt-node; + +ctxt-node = node; + if (!(def-service = virXMLPropString(node, service)) !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, _(Missing required service attribute in DNS
Re: [libvirt] [PATCH] network: fix problems with SRV
On 03/18/2014 01:09 AM, Martin Kletzander wrote: On Tue, Mar 18, 2014 at 12:07:17AM -0600, Laine Stump wrote: A patch submitted by Steven Malin last week pointed out a problem with libvirt's DNS SRV record configuration: https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html When searching for that message later, I found another series that had been posted by Guannan Ren back in 2012 that somehow slipped between the cracks: https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html That patch was very much out of date, but also pointed out some real problems. This patch fixes all the noted problems by refactoring virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then verifies those fixes by added several new records to the test case. Problems fixed: * both service and protocol now have an underscore (_) prepended on the commandline, as required by RFC2782. srv service='sip' protocol='udp' domain='example.com' target='tests.example.com' port='5060' priority='10' weight='150'/ before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150 after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150 * if domain wasn't specified in the srv element, the extra trailing . will no longer be added to the dnsmasq commandline. srv service='sip' protocol='udp' target='tests.example.com' port='5060' priority='10' weight='150'/ before: srv-host=sip.udp.,tests.example.com,5060,10,150 after: srv-host=_sip._udp,tests.example.com,5060,10,150 * when optional attributes aren't specified, the separating comma is also now not placed on the dnsmasq commandline. If optional attributes in the middle of the line are not specified, they are replaced with 0 in the commandline. srv service='sip' protocol='udp' target='tests.example.com' port='5060'/ before: srv-host=sip.udp.,tests.example.com,5060,, after: srv-host=_sip._udp,tests.example.com,5060 (actually this would have generated an error, because optional attributes weren't really optional). * As a safeguard, the parser checks for a leading _ on service and protocol, and fails if it is found. (Note that we can be guaranteed that no existing valid configuration will have this, since until now the parser had only allowed tcp or udp for protocol, and the bridge driver wasn't prepending _, making it a 100% certainty that there was no example of working SRV record use in the wild). That's valid for protocol, but not for service. For service there was a check for length only and it was not escaped at all. Even though there couldn't be any abuse for that, settings that resulted in generating invalid config file for dnsmasq were parsed and saved without any error from libvirt. Just to make sure what I wrote made sense: my point was that, due to the fact that the current code would only be able to generate records like this: [some-name].tcp.[some-domain] [some-name].udp.[some-domain] (i.e. the protocol could only be the exact string tcp or udp), and since the RFC *requires* that the protocol be prefixed with an underscore, it is impossible that anyone has an actual working config using the existing code (although it may have been saved with no error, it wouldn't do anything useful). Because it is impossible that it could have worked before, we don't need to worry about backward compatibility problems - nothing we do could break a working config. But I *think* what you're pointing out is that, although the config may not be working, it's possible that someone could have made a config like this: srv service='_sip' protocol='udp' domain='example.com' target='sip.example.com' port='5060'/ which they could have saved, even though it would do nothing useful, it would be saved. Then when that system was updated to the new libvirt, that network's config would generate an error while loading, so the network would be ignored. Gah!! So I *can't* add this new restriction in the parser. I could check for it during networkValidate() in the bridge driver, though. Do you think this is worthwhile? (I think I'll do it as a separate patch in any case). - -/* Check whether protocol value is the supported one */ -if (def-protocol STRNEQ(def-protocol, tcp) -(STRNEQ(def-protocol, udp))) { +if (def-protocol def-protocol[0] == '_') { virReportError(VIR_ERR_XML_DETAIL, I'm not sure we want to allow anything not starting with '_'. OTOH, we allowed anything in the service (even dots and commas and so on (non-working configuration). I was bothered by this as well, and spent quite awhile searching RFCs to get a definitive answer on what characters are allowed in a service name or a protocol name. I couldn't find this information, though. I could only find lists of currently assigned service/protocol names. I guess since it is the number that goes over the
Re: [libvirt] Zombie process after open libvirt connection
Thank you Michal, this is good news for me. I'll wait for this patch. Regards, -- Carlos Rodrigues Engenheiro de Software Sénior Eurotux Informática, S.A. | www.eurotux.com (t) +351 253 680 300 (m) +351 911 926 110 On Qua, 2014-03-19 at 18:27 +0100, Michal Privoznik wrote: On 19.03.2014 12:10, Carlos Rodrigues wrote: Hello Michal, I am using libvirt 1.1.3 and perl-Sys-Virt 1.1.3 and perl-5.16 on Fedora 19 x86_64 The zombie process appears after open libvirt connection with qemu-tls, and perl module is binding for libvirt library XS. Here is my running example with zombie process: $ perl test-chldhandle-bug-fixed.pl sleep 15 echo ps axf | grep perl echo [2] 12427 init... pid=12427 while... fork 1 end... pid=12430 receive chld fork 2 end... pid=12431 receive chld 2014-03-19 11:06:38.712+: 12427: info : libvirt version: 1.1.3.1, package: 2.fc19 (Unknown, 2014-03-17-15:02:00, cmar-laptop.lan) 2014-03-19 11:06:38.712+: 12427: warning : virNetTLSContextCheckCertificate:1140 : Certificate check failed Certificate [session] owner does not match the hostname 10.10.4.249 connection open fork 3 end... pid=12432 fork 4 end... pid=12440 12427 pts/2S 0:00 | \_ perl test-chldhandle-bug-fixed.pl 12432 pts/2Z 0:00 | | \_ [perl] defunct 12440 pts/2Z 0:00 | | \_ [perl] defunct 12442 pts/2S+ 0:00 | \_ grep --color=auto perl Aha! It seems like this is only present if using tls, I was unable to reproduce this with tcp or unix sockets. And when using tcp I can see SIGCHLD being delivered while with tls it is not. That makes me wonder if either libvirt or gnutls silently sets signal mask and not restore it back. Because if I take a look at signal mask I can clearly see SIGCHLD to be blocked (from /proc/$pid/status): SigPnd: ShdPnd: 0001 SigBlk: 08011000 SigIgn: 1080 SigCgt: 00018001 What we can see here is, SigBlk (the bitmask of blocked signals) contains 0x801100 which is SIGPIPE, SIGCHLD and SIGWINCH. Right, why would libvirt care about SIGWINCH anyway? Git greping it leads us to virNetClientSetTLSSession(). I can clearly see there we are adding just those three signals to a mask. Then setting this mask just prior to calling poll() and then restoring back. Oh wait, we are not! pthread_sigmask(SIG_BLOCK,...) is just adding new signals to the mask, not overwriting the old one. So yes, this is clearly libvirt bug. If I use SIG_SETMASK there, I am no longer getting any zombies. I'll post the patch shortly. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/18] conf: accessors for common source information
A future patch will split virDomainDiskDef, in order to track multiple host resources per guest disk. To reduce the size of that patch, I've factored out the four most common accesses into functions, so that I can incrementally upgrade the code base to use the accessors, and so that code that doesn't care about the distinction of per-file details won't have to be changed when the struct changes. * src/conf/domain_conf.h (virDomainDiskGetType) (virDomainDiskSetType, virDomainDiskGetSource) (virDomainDiskSetSource, virDomainDiskGetDriver) (virDomainDiskSetDriver, virDomainDiskGetFormat) (virDomainDiskSetFormat): New prototypes. * src/conf/domain_conf.c (virDomainDiskGetType) (virDomainDiskSetType, virDomainDiskGetSource) (virDomainDiskSetSource, virDomainDiskGetDriver) (virDomainDiskSetDriver, virDomainDiskGetFormat) (virDomainDiskSetFormat): Implement them. * src/libvirt_private.syms (domain_conf.h): Export them. Signed-off-by: Eric Blake ebl...@redhat.com --- src/conf/domain_conf.c | 72 src/conf/domain_conf.h | 10 +++ src/libvirt_private.syms | 8 ++ 3 files changed, 90 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89aa52c..d2724ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1355,6 +1355,20 @@ error: int +virDomainDiskGetType(virDomainDiskDefPtr def) +{ +return def-type; +} + + +void +virDomainDiskSetType(virDomainDiskDefPtr def, int type) +{ +def-type = type; +} + + +int virDomainDiskGetActualType(virDomainDiskDefPtr def) { if (def-type == VIR_DOMAIN_DISK_TYPE_VOLUME def-srcpool) @@ -1364,6 +1378,64 @@ virDomainDiskGetActualType(virDomainDiskDefPtr def) } +const char * +virDomainDiskGetSource(virDomainDiskDefPtr def) +{ +return def-src; +} + + +int +virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) +{ +int ret; +char *tmp = def-src; + +ret = VIR_STRDUP(def-src, src); +if (ret 0) +def-src = tmp; +else +VIR_FREE(tmp); +return ret; +} + + +const char * +virDomainDiskGetDriver(virDomainDiskDefPtr def) +{ +return def-driverName; +} + + +int +virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) +{ +int ret; +char *tmp = def-driverName; + +ret = VIR_STRDUP(def-driverName, name); +if (ret 0) +def-driverName = tmp; +else +VIR_FREE(tmp); +return ret; +} + + +int +virDomainDiskGetFormat(virDomainDiskDefPtr def) +{ +return def-format; +} + + +void +virDomainDiskSetFormat(virDomainDiskDefPtr def, int format) +{ +def-format = format; +} + + void virDomainControllerDefFree(virDomainControllerDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 27f07e6..cc447b0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2256,7 +2256,17 @@ void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts); virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts, virDomainDiskHostDefPtr hosts); +int virDomainDiskGetType(virDomainDiskDefPtr def); +void virDomainDiskSetType(virDomainDiskDefPtr def, int type); int virDomainDiskGetActualType(virDomainDiskDefPtr def); +const char *virDomainDiskGetSource(virDomainDiskDefPtr def); +int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) +ATTRIBUTE_RETURN_CHECK; +const char *virDomainDiskGetDriver(virDomainDiskDefPtr def); +int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) +ATTRIBUTE_RETURN_CHECK; +int virDomainDiskGetFormat(virDomainDiskDefPtr def); +void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format); int virDomainDeviceFindControllerModel(virDomainDefPtr def, virDomainDeviceInfoPtr info, int controllerType); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3baf766..624b420 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -200,6 +200,10 @@ virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; virDomainDiskGetActualType; +virDomainDiskGetDriver; +virDomainDiskGetFormat; +virDomainDiskGetSource; +virDomainDiskGetType; virDomainDiskHostDefClear; virDomainDiskHostDefCopy; virDomainDiskHostDefFree; @@ -214,6 +218,10 @@ virDomainDiskProtocolTransportTypeToString; virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSetDriver; +virDomainDiskSetFormat; +virDomainDiskSetSource; +virDomainDiskSetType; virDomainDiskSourceIsBlockType; virDomainDiskTypeFromString; virDomainDiskTypeToString; -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/18] conf: use disk source accessors in bhyve/
Part of a series of cleanups to use new accessor methods. * src/bhyve/bhyve_command.c (bhyveBuildDiskArgStr) (virBhyveProcessBuildLoadCmd): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com --- I haven't actually compile-tested this one yet, but unless my grep was off, I think I got it correctly. src/bhyve/bhyve_command.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 15029cd..cfb577c 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -214,14 +214,15 @@ bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; } -if (disk-type != VIR_DOMAIN_DISK_TYPE_FILE) { +if (virDomainDiskGetType(disk) != VIR_DOMAIN_DISK_TYPE_FILE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(unsupported disk type)); return -1; } virCommandAddArg(cmd, -s); -virCommandAddArgFormat(cmd, 2:0,%s,%s, bus_type, disk-src); +virCommandAddArgFormat(cmd, 2:0,%s,%s, bus_type, + virDomainDiskGetSource(disk)); return 0; } @@ -317,7 +318,7 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, return NULL; } -if (disk-type != VIR_DOMAIN_DISK_TYPE_FILE) { +if (virDomainDiskGetType(disk) != VIR_DOMAIN_DISK_TYPE_FILE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(unsupported disk type)); return NULL; @@ -332,7 +333,7 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, /* Image path */ virCommandAddArg(cmd, -d); -virCommandAddArg(cmd, disk-src); +virCommandAddArg(cmd, virDomainDiskGetSource(disk)); /* VM name */ virCommandAddArg(cmd, vm-def-name); -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/18] conf: use disk source accessors in vmware/
Part of a series of cleanups to use new accessor methods. While writing this, I also discovered that conversion from XML to vmware modified the disk source in place; if the same code is reached twice, the second call behaves differently because the first call didn't clean up its mess. * src/vmware/vmware_conf.c (vmwareVmxPath): Use accessors. (vmwareParsePath): Avoid munging input string. * src/vmware/vmware_conf.h (vmwareParsePath): Make static. Signed-off-by: Eric Blake ebl...@redhat.com --- src/vmware/vmware_conf.c | 18 ++ src/vmware/vmware_conf.h | 6 +++--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..2de24a7 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -331,15 +331,15 @@ vmwareDomainConfigDisplay(vmwareDomainPtr pDomain, virDomainDefPtr def) } } -int -vmwareParsePath(char *path, char **directory, char **filename) +static int +vmwareParsePath(const char *path, char **directory, char **filename) { char *separator; separator = strrchr(path, '/'); if (separator != NULL) { -*separator++ = '\0'; +separator++; if (*separator == '\0') { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -347,7 +347,7 @@ vmwareParsePath(char *path, char **directory, char **filename) return -1; } -if (VIR_STRDUP(*directory, path) 0) +if (VIR_STRNDUP(*directory, path, separator - path - 1) 0) goto error; if (VIR_STRDUP(*filename, separator) 0) { VIR_FREE(*directory); @@ -388,6 +388,7 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) char *fileName = NULL; int ret = -1; size_t i; +const char *src; /* * Build VMX URL. Use the source of the first file-based harddisk @@ -405,7 +406,7 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) for (i = 0; i vmdef-ndisks; ++i) { if (vmdef-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK -vmdef-disks[i]-type == VIR_DOMAIN_DISK_TYPE_FILE) { +virDomainDiskGetType(vmdef-disks[i]) == VIR_DOMAIN_DISK_TYPE_FILE) { disk = vmdef-disks[i]; break; } @@ -418,21 +419,22 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) goto cleanup; } -if (disk-src == NULL) { +src = virDomainDiskGetSource(disk); +if (!src) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(First file-based harddisk has no source, cannot deduce datastore and path for VMX file)); goto cleanup; } -if (vmwareParsePath(disk-src, directoryName, fileName) 0) { +if (vmwareParsePath(src, directoryName, fileName) 0) { goto cleanup; } if (!virFileHasSuffix(fileName, .vmdk)) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Expecting source '%s' of first file-based harddisk - to be a VMDK image), disk-src); + to be a VMDK image), src); goto cleanup; } diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index b9fca6c..b039f9e 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -1,5 +1,7 @@ /*---*/ -/* Copyright 2010, diateam (www.diateam.net) +/* + * Copyright (C) 2014, Red Hat, Inc. + * Copyright 2010, diateam (www.diateam.net) * Copyright (c) 2013, Doug Goldstein (car...@cardoe.com) * * This library is free software; you can redistribute it and/or @@ -71,8 +73,6 @@ int vmwareParseVersionStr(int type, const char *buf, unsigned long *version); int vmwareDomainConfigDisplay(vmwareDomainPtr domain, virDomainDefPtr vmdef); -int vmwareParsePath(char *path, char **directory, char **filename); - int vmwareConstructVmxPath(char *directoryName, char *name, char **vmxPath); -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Zombie process after open libvirt connection
On 19.03.2014 18:55, Carlos Rodrigues wrote: Thank you Michal, this is good news for me. I'll wait for this patch. Regards, I've just pushed it to the repository: commit 3d4b4f5ac634c123af1981084add29d3a2ca6ab0 Author: Michal Privoznik mpriv...@redhat.com AuthorDate: Wed Mar 19 18:10:34 2014 +0100 Commit: Michal Privoznik mpriv...@redhat.com CommitDate: Wed Mar 19 18:54:51 2014 +0100 virNetClientSetTLSSession: Restore original signal mask Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. Signed-off-by: Michal Privoznik mpriv...@redhat.com Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 4/8] Don't create iscsiadm command line in ISCSIPool{Start, Stop}
Create ISCSIConnection{Login,Logout} wrappers for that. --- src/storage/storage_backend_iscsi.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 20fc0e6..35c9cc5 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -356,6 +356,24 @@ cleanup: } static int +virStorageBackendISCSIConnectionLogin(const char *portal, + const char *initiatoriqn, + const char *target) +{ +const char *extraargv[] = { --login, NULL }; +return virStorageBackendISCSIConnection(portal, initiatoriqn, target, extraargv); +} + +static int +virStorageBackendISCSIConnectionLogout(const char *portal, + const char *initiatoriqn, + const char *target) +{ +const char *extraargv[] = { --logout, NULL }; +return virStorageBackendISCSIConnection(portal, initiatoriqn, target, extraargv); +} + +static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { @@ -789,7 +807,6 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, char *portal = NULL; char *session = NULL; int ret = -1; -const char *loginargv[] = { --login, NULL }; if (pool-def-source.nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, @@ -825,10 +842,9 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, if (virStorageBackendISCSISetAuth(portal, conn, pool-def) 0) goto cleanup; -if (virStorageBackendISCSIConnection(portal, - pool-def-source.initiator.iqn, - pool-def-source.devices[0].path, - loginargv) 0) +if (virStorageBackendISCSIConnectionLogin(portal, + pool-def-source.initiator.iqn, + pool-def-source.devices[0].path) 0) goto cleanup; } ret = 0; @@ -867,17 +883,15 @@ static int virStorageBackendISCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { -const char *logoutargv[] = { --logout, NULL }; char *portal; int ret = -1; if ((portal = virStorageBackendISCSIPortal(pool-def-source)) == NULL) return -1; -if (virStorageBackendISCSIConnection(portal, - pool-def-source.initiator.iqn, - pool-def-source.devices[0].path, - logoutargv) 0) +if (virStorageBackendISCSIConnectionLogout(portal, + pool-def-source.initiator.iqn, + pool-def-source.devices[0].path) 0) goto cleanup; ret = 0; -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix virQEMUCapsLoadCache leaks
Valgrind reported leaking of maxCpus and arch strings from virXPathString, as well as the leak of the machineMaxCpus array. Use 'tmp' for the strings we don't want to free, to allow freeing of 'str' in the cleanup label and free machineMaxCpus in virCapsReset too. --- src/qemu/qemu_capabilities.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2914200..e742c03 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2376,7 +2376,8 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, int n; xmlNodePtr *nodes = NULL; xmlXPathContextPtr ctxt = NULL; -char *str; +char *str = NULL; +char *tmp; long long int l; if (!(doc = virXMLParseFile(filename))) @@ -2432,7 +2433,6 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, if (flag 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown qemu capabilities flag %s), str); -VIR_FREE(str); goto cleanup; } VIR_FREE(str); @@ -2463,6 +2463,7 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, _(unknown arch %s in QEMU capabilities cache), str); goto cleanup; } +VIR_FREE(str); if ((n = virXPathNodeSet(./cpu, ctxt, nodes)) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -2476,12 +2477,12 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, goto cleanup; for (i = 0; i n; i++) { -if (!(str = virXMLPropString(nodes[i], name))) { +if (!(tmp = virXMLPropString(nodes[i], name))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(missing cpu name in QEMU capabilities cache)); goto cleanup; } -qemuCaps-cpuDefinitions[i] = str; +qemuCaps-cpuDefinitions[i] = tmp; } } VIR_FREE(nodes); @@ -2503,12 +2504,12 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, goto cleanup; for (i = 0; i n; i++) { -if (!(str = virXMLPropString(nodes[i], name))) { +if (!(tmp = virXMLPropString(nodes[i], name))) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(missing machine name in QEMU capabilities cache)); goto cleanup; } -qemuCaps-machineTypes[i] = str; +qemuCaps-machineTypes[i] = tmp; qemuCaps-machineAliases[i] = virXMLPropString(nodes[i], alias); @@ -2519,12 +2520,14 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, _(malformed machine cpu count in QEMU capabilities cache)); goto cleanup; } +VIR_FREE(str); } } VIR_FREE(nodes); ret = 0; - cleanup: +cleanup: +VIR_FREE(str); VIR_FREE(nodes); xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); @@ -2668,6 +2671,7 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps) } VIR_FREE(qemuCaps-machineTypes); VIR_FREE(qemuCaps-machineAliases); +VIR_FREE(qemuCaps-machineMaxCpus); qemuCaps-nmachineTypes = 0; } -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 8/8] Add test for virISCSIScanTargets
--- tests/viriscsitest.c | 82 1 file changed, 82 insertions(+) diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c index 0b09834..16a40c7 100644 --- a/tests/viriscsitest.c +++ b/tests/viriscsitest.c @@ -51,6 +51,14 @@ static const char *iscsiadmSessionOutputNonFlash = tcp: [6] 10.20.30.44:3260,1 iqn.2008-04.example:example1:iscsi.bar (non-flash)\n tcp: [7] 10.20.30.45:3260,1 iqn.2009-04.example:example1:iscsi.seven (non-flash)\n; +const char *iscsiadmSendtargetsOutput = +10.20.30.40:3260,1 iqn.2004-06.example:example1:iscsi.test\n +10.20.30.40:3260,1 iqn.2005-05.example:example1:iscsi.hello\n +10.20.30.40:3260,1 iqn.2006-04.example:example1:iscsi.world\n +10.20.30.40:3260,1 iqn.2007-04.example:example1:iscsi.foo\n +10.20.30.40:3260,1 iqn.2008-04.example:example1:iscsi.bar\n +10.20.30.40:3260,1 iqn.2009-04.example:example1:iscsi.seven\n; + struct testSessionInfo { const char *device_path; int output_version; @@ -74,6 +82,15 @@ static void testIscsiadmCb(const char *const*args, ignore_value(VIR_STRDUP(*output, iscsiadmSessionOutputNonFlash)); else ignore_value(VIR_STRDUP(*output, iscsiadmSessionOutput)); +} else if (args[0] STREQ(args[0], ISCSIADM) + args[1] STREQ(args[1], --mode) + args[2] STREQ(args[2], discovery) + args[3] STREQ(args[3], --type) + args[4] STREQ(args[4], sendtargets) + args[5] STREQ(args[5], --portal) + args[6] STREQ(args[6], 10.20.30.40:3260,1) + args[7] == NULL) { +ignore_value(VIR_STRDUP(*output, iscsiadmSendtargetsOutput)); } else { *status = -1; } @@ -107,6 +124,54 @@ cleanup: return ret; } +struct testScanTargetsInfo { +const char *fake_cmd_output; +const char *portal; +const char **expected_targets; +size_t nexpected; +}; + +static int +testISCSIScanTargets(const void *data) +{ +const struct testScanTargetsInfo *info = data; +size_t ntargets = 0; +char **targets = NULL; +int ret = -1; +size_t i; + +virCommandSetDryRun(NULL, testIscsiadmCb, NULL); + +if (virISCSIScanTargets(info-portal, NULL, +ntargets, targets) 0) +goto cleanup; + +if (info-nexpected != ntargets) { +virReportError(VIR_ERR_INTERNAL_ERROR, + Expected %zu targets, got %zu, + info-nexpected, ntargets); +goto cleanup; +} + +for (i = 0; i ntargets; i++) { +if (STRNEQ(info-expected_targets[i], targets[i])) { +virReportError(VIR_ERR_INTERNAL_ERROR, + Expected target '%s', got '%s', + info-expected_targets[i], targets[i]); +goto cleanup; +} +} + +ret = 0; + +cleanup: +virCommandSetDryRun(NULL, NULL, NULL); +for (i = 0; i ntargets; i++) +VIR_FREE(targets[i]); +VIR_FREE(targets); +return ret; +} + static int mymain(void) { @@ -128,6 +193,23 @@ mymain(void) DO_SESSION_TEST(iqn.2009-04.example:example1:iscsi.seven, 7); DO_SESSION_TEST(iqn.2009-04.example:example1:iscsi.eight, NULL); +const char *targets[] = { +iqn.2004-06.example:example1:iscsi.test, +iqn.2005-05.example:example1:iscsi.hello, +iqn.2006-04.example:example1:iscsi.world, +iqn.2007-04.example:example1:iscsi.foo, +iqn.2008-04.example:example1:iscsi.bar, +iqn.2009-04.example:example1:iscsi.seven +}; +struct testScanTargetsInfo infoTargets = { +.fake_cmd_output = iscsiadm_sendtargets, +.portal = 10.20.30.40:3260,1, +.expected_targets = targets, +.nexpected = ARRAY_CARDINALITY(targets), +}; +if (virtTestRun(ISCSI scan targets, testISCSIScanTargets, infoTargets) 0) +rv = -1; + if (rv 0) return EXIT_FAILURE; return EXIT_SUCCESS; -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 5/8] Remove storage pool from the arguments of a few functions
virStorageBackendISCSISession only needs the path of the source device and virStorageBackendISCSIRescanLUNs doesn't need the pool at all. This will allow the functions to be moved to src/util. --- src/storage/storage_backend_iscsi.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 35c9cc5..b7a0380 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -96,8 +96,8 @@ virStorageBackendISCSIExtractSession(char **const groups, } static char * -virStorageBackendISCSISession(virStoragePoolObjPtr pool, - bool probe) +virStorageBackendISCSIGetSession(const char *devpath, + bool probe) { /* * # iscsiadm --mode session @@ -114,7 +114,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool, }; struct virStorageBackendISCSISessionData cbdata = { .session = NULL, -.devpath = pool-def-source.devices[0].path +.devpath = devpath, }; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, --mode, session, NULL); @@ -138,6 +138,13 @@ cleanup: return cbdata.session; } +static char * +virStorageBackendISCSISession(virStoragePoolObjPtr pool, + bool probe) +{ +return virStorageBackendISCSIGetSession(pool-def-source.devices[0].path, probe); +} + #define LINE_SIZE 4096 @@ -442,8 +449,7 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, } static int -virStorageBackendISCSIRescanLUNs(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, - const char *session) +virStorageBackendISCSIRescanLUNs(const char *session) { virCommandPtr cmd = virCommandNewArgList(ISCSIADM, --mode, session, @@ -865,7 +871,7 @@ virStorageBackendISCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if ((session = virStorageBackendISCSISession(pool, false)) == NULL) goto cleanup; -if (virStorageBackendISCSIRescanLUNs(pool, session) 0) +if (virStorageBackendISCSIRescanLUNs(session) 0) goto cleanup; if (virStorageBackendISCSIFindLUs(pool, session) 0) goto cleanup; -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 7/8] Add test for virISCSIGetSession
Parse iscsiadm output with and without the recently introduced flashnode info. [1] This should check that commits like 57e17a7 (fixing [2]) do not break iscsiadm output parsing. [1] https://github.com/mikechristie/open-iscsi/commit/181af9a [2] https://bugzilla.redhat.com/show_bug.cgi?id=1067173 --- tests/Makefile.am| 6 +++ tests/viriscsitest.c | 137 +++ 2 files changed, 143 insertions(+) create mode 100644 tests/viriscsitest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 90f70ff..a1a89ab 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -141,6 +141,7 @@ test_programs = virshtest sockettest \ virendiantest \ virfiletest \ viridentitytest \ + viriscsitest \ virkeycodetest \ virlockspacetest \ virlogtest \ @@ -643,6 +644,7 @@ storagevolxml2argvtest_SOURCES = \ testutils.c testutils.h storagevolxml2argvtest_LDADD = \ ../src/libvirt_driver_storage_impl.la $(LDADDS) + else ! WITH_STORAGE EXTRA_DIST += storagevolxml2argvtest.c endif ! WITH_STORAGE @@ -787,6 +789,10 @@ viridentitytest_DEPENDENCIES = libsecurityselinuxhelper.la \ ../src/libvirt.la endif WITH_SELINUX +viriscsitest_SOURCES = \ + viriscsitest.c testutils.h testutils.c +viriscsitest_LDADD = $(LDADDS) + virkeycodetest_SOURCES = \ virkeycodetest.c testutils.h testutils.c virkeycodetest_LDADD = $(LDADDS) diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c new file mode 100644 index 000..0b09834 --- /dev/null +++ b/tests/viriscsitest.c @@ -0,0 +1,137 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Jan Tomko jto...@redhat.com + */ + +#include config.h + +#ifdef WIN32 +int +main(void) +{ +return EXIT_AM_SKIP; +} +#else +# define __VIR_COMMAND_PRIV_H_ALLOW__ + +# include testutils.h +# include vircommandpriv.h +# include viriscsi.h + +# define VIR_FROM_THIS VIR_FROM_NONE + +static const char *iscsiadmSessionOutput = +tcp: [1] 10.20.30.40:3260,1 iqn.2004-06.example:example1:iscsi.test\n +tcp: [2] 10.20.30.41:3260,1 iqn.2005-05.example:example1:iscsi.hello\n +tcp: [3] 10.20.30.42:3260,1 iqn.2006-04.example:example1:iscsi.world\n +tcp: [5] 10.20.30.43:3260,1 iqn.2007-04.example:example1:iscsi.foo\n +tcp: [6] 10.20.30.44:3260,1 iqn.2008-04.example:example1:iscsi.bar\n +tcp: [7] 10.20.30.45:3260,1 iqn.2009-04.example:example1:iscsi.seven\n; + +static const char *iscsiadmSessionOutputNonFlash = +tcp: [1] 10.20.30.40:3260,1 iqn.2004-06.example:example1:iscsi.test (non-flash)\n +tcp: [2] 10.20.30.41:3260,1 iqn.2005-05.example:example1:iscsi.hello (non-flash)\n +tcp: [3] 10.20.30.42:3260,1 iqn.2006-04.example:example1:iscsi.world (non-flash)\n +tcp: [5] 10.20.30.43:3260,1 iqn.2007-04.example:example1:iscsi.foo (non-flash)\n +tcp: [6] 10.20.30.44:3260,1 iqn.2008-04.example:example1:iscsi.bar (non-flash)\n +tcp: [7] 10.20.30.45:3260,1 iqn.2009-04.example:example1:iscsi.seven (non-flash)\n; + +struct testSessionInfo { +const char *device_path; +int output_version; +const char *expected_session; +}; + +static void testIscsiadmCb(const char *const*args, + const char *const*env ATTRIBUTE_UNUSED, + const char *input ATTRIBUTE_UNUSED, + char **output, + char **error ATTRIBUTE_UNUSED, + int *status, + void *opaque) +{ +int *output_version = opaque; +if (args[0] STREQ(args[0], ISCSIADM) +args[1] STREQ(args[1], --mode) +args[2] STREQ(args[2], session) +args[3] == NULL) { +if (*output_version == 1) +ignore_value(VIR_STRDUP(*output, iscsiadmSessionOutputNonFlash)); +else +ignore_value(VIR_STRDUP(*output, iscsiadmSessionOutput)); +} else { +*status = -1; +} +} + +static int +testISCSIGetSession(const void *data) +{ +const struct testSessionInfo *info = data; +int ver = info-output_version; +char *actual_session = NULL; +int ret = -1; + +virCommandSetDryRun(NULL, testIscsiadmCb, ver); + +actual_session = virISCSIGetSession(info-device_path, true); + +if (STRNEQ_NULLABLE(actual_session, info-expected_session)) { +
[libvirt] Adding support to limit client connections for vnc/spice display driver
Hello, Greeting!!! We are using KVM hypervisor driver for running OpenStack IaaS. Couple of months back we have reported one security issue [1] in OS. Basically we want to limit on the number of vnc client connections that can be opened by users for a given VM. I know that there is share policy where you can specify VNC_SHARE_MODE_EXCLUSIVE share mode. But it only allows one client connection to the vnc console, it will disconnect all previously opened vnc client connections. Since vnc display driver already has the data of client connections, it would be possible to add the logic to limit on the number of client connections. What we want is the ability to specify threshold how many vnc client connections should be allowed at any given point of time in the domain xml for graphics device (especially for vnc/spice)? Example graphics type=vnc autoport=yes keymap=en-us listen=127.0.0.1/, share-policy=limit-connections, connections=5 Add support to accept new share policy limit-connections'. So in the above example, when user tries to open vnc display for the 6th time, then the request should be rejected. I need your expert opinion in deciding whether it's a good idea to add this support in the vnc/spice display driver or this kind of constraint should be imposed outside libvirt? [1] : https://bugs.launchpad.net/nova/+bug/1227575 Tushar __ Disclaimer:This email and any attachments are sent in strictest confidence for the sole use of the addressee and may contain legally privileged, confidential, and proprietary data. If you are not the intended recipient, please advise the sender by replying promptly to this email and then delete and destroy this email and any attachments without any further use, copying or forwarding -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] is_selinux_enabled returns -1 on error, account for this.
On 03/18/2014 01:02 PM, Scott Sullivan wrote: Per the documentation, is_selinux_enabled() returns -1 on error. Account for this. Previously when -1 was being returned the condition would still be true. I was noticing this because on my system that has selinux disabled I was getting this in the libvirt.log every 5 seconds: error : virIdentityGetSystem:173 : Unable to lookup SELinux process context: Invalid argument With this patch applied, I no longer get these messages every 5 seconds. I am submitting this in case its deemed useful for inclusion. Anyone have any comments on this change? This is a patch off current master. From 23e0780db43ebd3ea90710750639df901c261674 Mon Sep 17 00:00:00 2001 From: Scott Sullivan ssulli...@liquidweb.com Date: Tue, 18 Mar 2014 12:55:50 -0400 Subject: [PATCH] is_selinux_enabled returns -1 on error, account for this. --- src/security/security_selinux.c |2 +- src/util/viridentity.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 02c7496..5f46bef 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -784,7 +784,7 @@ error: static int virSecuritySELinuxSecurityDriverProbe(const char *virtDriver) { -if (!is_selinux_enabled()) +if (is_selinux_enabled() = 0) return SECURITY_DRIVER_DISABLE; if (virtDriver STREQ(virtDriver, LXC)) { diff --git a/src/util/viridentity.c b/src/util/viridentity.c index 351fdd7..05e7568 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -169,7 +169,7 @@ virIdentityPtr virIdentityGetSystem(void) goto cleanup; #if WITH_SELINUX -if (is_selinux_enabled()) { +if (is_selinux_enabled() 0) { if (getcon(con) 0) { virReportSystemError(errno, %s, _(Unable to lookup SELinux process context)); ping? Looking for a ACK/NACK on this from a committer. In the case of an error condition when calling is_selinux_enabled() it seems safer to assume seLinux isn't enabled than to assume it is. If you assume its enabled like it is in master, at least one result is Unable to lookup SELinux process context spewed into libvirt.log many times a minute on my systems causing the file to grow large, and needless IO. On my systems that do exhibit this behavior (CentOS 6), I show seLinux as disabled: [root@host ~]# sestatus SELinux status: disabled [root@host ~]# -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] LSN-2013-0021: libvirtd crash during seamless SPICE migration
[this is an older issue, but it was just barely assigned a CVE this week] Libvirt Security Notice: LSN-2013-0021 == Summary: libvirtd crash during seamless SPICE migration Reported on: 20130919 Published on: 20130919 Fixed on: 20130920 Reported by: Marian Krcmarik mkrcm...@redhat.com Patched by: Martin Kletzander mklet...@redhat.com See also: CVE-2013-7336 Description --- When migrating a guest with a live SPICE connection, the source libvirtd did not properly track that the migration job was still waiting for status from the handshakes involved in seamless migration. Impact -- If another client was querying domain status at the same time as the ongoing seamless SPICE migration, the incorrect job status could lead to memory corruption and a crash of libvirtd on the source side of the migration. As queries can be performed by an unprivileged user, this can be used to inflict a denial of service attack on other users of the libvirtd daemon with higher privilege. Workaround -- The impact can be mitigated by blocking access to the read-only libvirtd UNIX domain socket, with policykit or the 'auth_unix_ro' parameter in '/etc/libvirt/libvirtd.conf'. If ACLs are active, the 'read' permission should be removed from any untrusted users. This will not prevent the crash, but will stop unprivileged users from inflicting the denial of service on higher privileged users. Additionally, avoiding SPICE seamless migration is sufficient to avoid the problem. Affected product Name: libvirt Repository: git://libvirt.org/git/libvirt.git http://libvirt.org/git/?p=libvirt.git Branch: master Broken in: v1.1.0 Broken in: v1.1.1 Broken in: v1.1.2 Fixed in: v1.1.3 Broken by: 9da7b11bcd3e9732dd881a9e6158a0c98bafd9fe Fixed by: 484cc3217b73b865f00bf42a9c12187b37200699 Branch: v1.1.0-maint Broken by: 9da7b11bcd3e9732dd881a9e6158a0c98bafd9fe Fixed by: 476d0e38af11f3ff50d85e3f7aecad4cd8208c76 Branch: v1.1.1-maint Broken by: 9da7b11bcd3e9732dd881a9e6158a0c98bafd9fe Fixed by: fea2550974137918c2bc9e01f3eb00421585450c Branch: v1.1.2-maint Broken by: 9da7b11bcd3e9732dd881a9e6158a0c98bafd9fe Fixed by: b6ea7abcf72d7d0aaf90e17aa8e8e88db8f778ea -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Why using AC_PATH_PROG to detect the location of run-time external programs
Hello, I am wondering why AC_PATH_PROG is used to detect the location of run-time external programs (such as dnsmasq, ovs-vsctl, etc.), and hardcoded the locations in binary. It seems wrong to me. At compile-time, configure script should only determine whether the resulting binary should support the use of these external programs. The actual location of these programs should be determined at run-time, either from libvirtd.conf or by $PATH. I haven't looked into how to fix it. The problem I ran into earlier is this: on my dev box, I have Open vSwitch utility ovs-vsctl installed locally in /usr/local/bin. Therefore, at build-time, libvirt detected the location of ovs-vsctl and hardcoded '/usr/local/bin/ovs-vsctl' in libvirtd. When I package libvirt and install it on my test box, it couldn't find ovs-vsctl, as ovs-vsctl is installed at /usr/bin/ovs-vsctl in the test box, so it failed. Thanks, Howard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 6/8] Move functions using iscsiadm to viriscsi.c
Remove the 'StorageBackend' from names of the functions and fix indentation. --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms| 9 + src/storage/storage_backend_iscsi.c | 498 ++-- src/storage/storage_backend_iscsi.h | 4 - src/util/viriscsi.c | 498 src/util/viriscsi.h | 52 7 files changed, 588 insertions(+), 475 deletions(-) create mode 100644 src/util/viriscsi.c create mode 100644 src/util/viriscsi.h diff --git a/po/POTFILES.in b/po/POTFILES.in index efac7b2..5a4112a 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -168,6 +168,7 @@ src/util/virhostdev.c src/util/viridentity.c src/util/virinitctl.c src/util/viriptables.c +src/util/viriscsi.c src/util/virjson.c src/util/virkeyfile.c src/util/virlockspace.c diff --git a/src/Makefile.am b/src/Makefile.am index 4fdd871..55427ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -113,6 +113,7 @@ UTIL_SOURCES = \ util/viridentity.c util/viridentity.h \ util/virinitctl.c util/virinitctl.h \ util/viriptables.c util/viriptables.h \ + util/viriscsi.c util/viriscsi.h \ util/virjson.c util/virjson.h \ util/virkeycode.c util/virkeycode.h \ util/virkeyfile.c util/virkeyfile.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c7e024d..d72a9af 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1359,6 +1359,15 @@ iptablesRemoveUdpInput; iptablesRemoveUdpOutput; +# util/viriscsi.h +virISCSIConnectionLogin; +virISCSIConnectionLogout; +virISCSIGetSession; +virISCSINodeUpdate; +virISCSIRescanLUNs; +virISCSIScanTargets; + + # util/virjson.h virJSONValueArrayAppend; virJSONValueArrayGet; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index b7a0380..7e7ffad 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -26,8 +26,6 @@ #include dirent.h #include sys/wait.h #include string.h -#include stdio.h -#include regex.h #include fcntl.h #include unistd.h #include sys/stat.h @@ -40,9 +38,9 @@ #include vircommand.h #include virerror.h #include virfile.h +#include viriscsi.h #include virlog.h #include virobject.h -#include virrandom.h #include virstring.h #include viruuid.h @@ -79,306 +77,14 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) return portal; } -struct virStorageBackendISCSISessionData { -char *session; -const char *devpath; -}; - -static int -virStorageBackendISCSIExtractSession(char **const groups, - void *opaque) -{ -struct virStorageBackendISCSISessionData *data = opaque; - -if (STREQ(groups[1], data-devpath)) -return VIR_STRDUP(data-session, groups[0]); -return 0; -} - -static char * -virStorageBackendISCSIGetSession(const char *devpath, - bool probe) -{ -/* - * # iscsiadm --mode session - * tcp: [1] 192.168.122.170:3260,1 demo-tgt-b - * tcp: [2] 192.168.122.170:3260,1 demo-tgt-a - * - * Pull out 2nd and 4th fields - */ -const char *regexes[] = { -^tcp:\\s+\\[(\\S+)\\]\\s+\\S+\\s+(\\S+).*$ -}; -int vars[] = { -2, -}; -struct virStorageBackendISCSISessionData cbdata = { -.session = NULL, -.devpath = devpath, -}; - -virCommandPtr cmd = virCommandNewArgList(ISCSIADM, --mode, session, NULL); - -if (virCommandRunRegex(cmd, - 1, - regexes, - vars, - virStorageBackendISCSIExtractSession, - cbdata, NULL) 0) -goto cleanup; - -if (cbdata.session == NULL !probe) { -virReportError(VIR_ERR_INTERNAL_ERROR, - %s, _(cannot find session)); -goto cleanup; -} - -cleanup: -virCommandFree(cmd); -return cbdata.session; -} static char * virStorageBackendISCSISession(virStoragePoolObjPtr pool, bool probe) { -return virStorageBackendISCSIGetSession(pool-def-source.devices[0].path, probe); -} - - -#define LINE_SIZE 4096 - -static int -virStorageBackendIQNFound(const char *initiatoriqn, - char **ifacename) -{ -int ret = IQN_MISSING, fd = -1; -char ebuf[64]; -FILE *fp = NULL; -char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL; -virCommandPtr cmd = virCommandNewArgList(ISCSIADM, - --mode, iface, NULL); - -if (VIR_ALLOC_N(line, LINE_SIZE) != 0) { -
[libvirt] [PATCHv2 2/8] Move virStorageBackendRun to vircommand
The only storage-specific parameter is the pool object, which is only used for passing to the callback function. --- src/libvirt_private.syms | 2 + src/storage/storage_backend.c | 249 -- src/storage/storage_backend.h | 22 --- src/storage/storage_backend_disk.c| 43 +++--- src/storage/storage_backend_fs.c | 9 +- src/storage/storage_backend_iscsi.c | 54 src/storage/storage_backend_logical.c | 63 + src/util/vircommand.c | 245 + src/util/vircommand.h | 20 +++ 9 files changed, 360 insertions(+), 347 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3baf766..c7e024d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1113,6 +1113,8 @@ virCommandRawStatus; virCommandRequireHandshake; virCommandRun; virCommandRunAsync; +virCommandRunNul; +virCommandRunRegex; virCommandSetAppArmorProfile; virCommandSetDryRun; virCommandSetErrorBuffer; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d14e633..b1421ec 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1632,252 +1632,3 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, return stablepath; } - - -#ifndef WIN32 -/* - * Run an external program. - * - * Read its output and apply a series of regexes to each line - * When the entire set of regexes has matched consecutively - * then run a callback passing in all the matches - */ -int -virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, - virCommandPtr cmd, - int nregex, - const char **regex, - int *nvars, - virStorageBackendListVolRegexFunc func, - void *data, const char *prefix) -{ -int fd = -1, err, ret = -1; -FILE *list = NULL; -regex_t *reg; -regmatch_t *vars = NULL; -char line[1024]; -int maxReg = 0; -size_t i, j; -int totgroups = 0, ngroup = 0, maxvars = 0; -char **groups; - -/* Compile all regular expressions */ -if (VIR_ALLOC_N(reg, nregex) 0) -return -1; - -for (i = 0; i nregex; i++) { -err = regcomp(reg[i], regex[i], REG_EXTENDED); -if (err != 0) { -char error[100]; -regerror(err, reg[i], error, sizeof(error)); -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Failed to compile regex %s), error); -for (j = 0; j i; j++) -regfree(reg[j]); -VIR_FREE(reg); -return -1; -} - -totgroups += nvars[i]; -if (nvars[i] maxvars) -maxvars = nvars[i]; - -} - -/* Storage for matched variables */ -if (VIR_ALLOC_N(groups, totgroups) 0) -goto cleanup; -if (VIR_ALLOC_N(vars, maxvars+1) 0) -goto cleanup; - -virCommandSetOutputFD(cmd, fd); -if (virCommandRunAsync(cmd, NULL) 0) { -goto cleanup; -} - -if ((list = VIR_FDOPEN(fd, r)) == NULL) { -virReportError(VIR_ERR_INTERNAL_ERROR, - %s, _(cannot read fd)); -goto cleanup; -} - -while (fgets(line, sizeof(line), list) != NULL) { -char *p = NULL; -/* Strip trailing newline */ -int len = strlen(line); -if (len line[len-1] == '\n') -line[len-1] = '\0'; - -/* ignore any command prefix */ -if (prefix) -p = STRSKIP(line, prefix); -if (!p) -p = line; - -for (i = 0; i = maxReg i nregex; i++) { -if (regexec(reg[i], p, nvars[i]+1, vars, 0) == 0) { -maxReg++; - -if (i == 0) -ngroup = 0; - -/* NULL terminate each captured group in the line */ -for (j = 0; j nvars[i]; j++) { -/* NB vars[0] is the full pattern, so we offset j by 1 */ -p[vars[j+1].rm_eo] = '\0'; -if (VIR_STRDUP(groups[ngroup++], p + vars[j+1].rm_so) 0) -goto cleanup; -} - -/* We're matching on the last regex, so callback time */ -if (i == (nregex-1)) { -if (((*func)(pool, groups, data)) 0) -goto cleanup; - -/* Release matches restart to matching the first regex */ -for (j = 0; j totgroups; j++) -VIR_FREE(groups[j]); -maxReg = 0; -ngroup = 0; -} -} -} -} - -ret = virCommandWait(cmd, NULL); -cleanup: -if (groups) { -for (j = 0; j totgroups; j++) -VIR_FREE(groups[j]); -VIR_FREE(groups); -
Re: [libvirt] Why using AC_PATH_PROG to detect the location of run-time external programs
On 03/19/2014 03:15 PM, Howard Tsai wrote: Hello, I am wondering why AC_PATH_PROG is used to detect the location of run-time external programs (such as dnsmasq, ovs-vsctl, etc.), and hardcoded the locations in binary. It seems wrong to me. It is only hard-coding the default location of the tools. You are free to override that, by using ./configure OVSVSCTL=/path/to/the/destinat*ion/ovs-vsctl to force the binary to be built with the hard-coded location you plan on using in your installation, or use ./configure OVSVSCTL=ovs-vsctl to force libvirtd to do a PATH lookup every time. At compile-time, configure script should only determine whether the resulting binary should support the use of these external programs. The actual location of these programs should be determined at run-time, either from libvirtd.conf or by $PATH. Deferring the decision to libvirtd.conf might be feasible, albeit more complex. Propose a patch, and we'll look at it. But for now, the default is good enough, and over-ridable at configure time, so it's not something that I'm personally bothered by enough to write a patch myself. By the way, we DO prefer hard-coded locations over PATH lookups, because PATH lookups can result in odd behavior when two people run qemu:///session with different PATH, and where the lookup may pick up a different binary for the two users. Configuring a hard-coded location requires a bit more planning, but is more secure - and since MOST users go with pre-built distributions, the hard-coded binaries can be guaranteed* to be present (for example, the rpm files used by Fedora guarantee that for any tool where we hard-code the location into the libvirt binary as living in /usr/bin will also be a prereq of the package, so that if you install libvirt, you also install the prereq packages that libvirt will try to use). [*Of course, if you ever hit a case where this guarantee is not met, file a bug against the distro to inform them of the missing dependency] I haven't looked into how to fix it. The problem I ran into earlier is this: on my dev box, I have Open vSwitch utility ovs-vsctl installed locally in /usr/local/bin. Therefore, at build-time, libvirt detected the location of ovs-vsctl and hardcoded '/usr/local/bin/ovs-vsctl' in libvirtd. When I package libvirt and install it on my test box, it couldn't find ovs-vsctl, as ovs-vsctl is installed at /usr/bin/ovs-vsctl in the test box, so it failed. Again, that's a matter of setting up OVSVSCTL on your dev box to match the location where the binary will be present on your test box. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 00/13] libxl: add basic support for migration
Michal Privoznik wrote: On 13.03.2014 23:11, Jim Fehlig wrote: V2 of https://www.redhat.com/archives/libvir-list/2014-March/msg00156.html New in this version: not much. I rebased on the hostdev passthrough changes and added a few 'begin phase' checks to fail early if the domain is not migratable. See 13/13 for details. Based on an earlier patch from Chunyan Liu https://www.redhat.com/archives/libvir-list/2013-September/msg00667.html This patch series adds basic migration support to the libxl driver. Follow-up patches can improve pre-migration checks and add support for additional migration flags. Patches 1-12 are almost exclusively code motion, moving functions from the main driver module into the libxl_domain and libxl_conf modules. Patch 13 contains the actual migration impl. Jim Fehlig (13): libxl: move libxlDomainEventQueue to libxl_domain libxl: move libxlDomainManagedSavePath to libxl_domain libxl: move libxlSaveImageOpen to libxl_domain libxl: move libxlVmCleanup{,Job} to libxl_domain libxl: move libxlDomEventsRegister to libxl_domain libxl: move libxlDomainAutoCoreDump to libxl_domain libxl: move libxlDoNodeGetInfo to libxl_conf libxl: move libxlDomainSetVcpuAffinities to libxl_domain libxl: move libxlFreeMem to libxl_domain libxl: move libxlVmStart to libxl_domain libxl: include a pointer to the driver in libxlDomainObjPrivate libxl: move domain event handler to libxl_domain libxl: add migration support po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/libxl/libxl_conf.c | 36 ++ src/libxl/libxl_conf.h | 10 + src/libxl/libxl_domain.c| 705 +++ src/libxl/libxl_domain.h| 51 ++- src/libxl/libxl_driver.c| 988 +++- src/libxl/libxl_migration.c | 598 +++ src/libxl/libxl_migration.h | 78 9 files changed, 1716 insertions(+), 754 deletions(-) create mode 100644 src/libxl/libxl_migration.c create mode 100644 src/libxl/libxl_migration.h ACK series. The first 12 patches are pure code movement (except for 11/13 which is trivial) and therefore can be pushed right now. Thanks, 1-12 pushed. I'll send a V3 of 13 addressing your comments. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 13/13] libxl: add migration support
Michal Privoznik wrote: On 13.03.2014 23:11, Jim Fehlig wrote: This patch adds initial migration support to the libxl driver, using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration functions. Signed-off-by: Jim Fehlig jfeh...@suse.com --- V2: - Now that the libxl driver supports hostdev passthrough, ensure domain has no hostdevs before migration - Fail early in libxlDomainMigrateBegin3Params if domain is inactive - Change name of local variable in libxlDoMigrateSend from 'live' to 'xl_flags'. po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/libxl/libxl_conf.h | 6 + src/libxl/libxl_domain.h| 1 + src/libxl/libxl_driver.c| 225 + src/libxl/libxl_migration.c | 598 src/libxl/libxl_migration.h | 78 ++ 7 files changed, 911 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3306dc1..6fc5266 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4313,6 +4323,216 @@ cleanup: return ret; } +static char * +libxlDomainMigrateBegin3Params(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ +const char *xmlin = NULL; +const char *dname = NULL; +virDomainObjPtr vm = NULL; + +virCheckFlags(LIBXL_MIGRATION_FLAGS, NULL); +if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) 0) +return NULL; + +if (virTypedParamsGetString(params, nparams, +VIR_MIGRATE_PARAM_DEST_XML, +xmlin) 0 || +virTypedParamsGetString(params, nparams, +VIR_MIGRATE_PARAM_DEST_NAME, +dname) 0) I'd expect @dname to be used somewhere... Seems I followed the qemu driver too closely, where AFAICT dname isn't used in the begin phase either. Is there even anything to do with dname in the begin phase on the migration source? +return NULL; + +if (!(vm = libxlDomObjFromDomain(domain))) +return NULL; + +if (virDomainMigrateBegin3ParamsEnsureACL(domain-conn, vm-def) 0) { +virObjectUnlock(vm); +return NULL; +} + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +virObjectUnlock(vm); +return NULL; +} + +return libxlDomainMigrationBegin(domain-conn, vm, xmlin); +} + diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c new file mode 100644 index 000..01023db --- /dev/null +++ b/src/libxl/libxl_migration.c @@ -0,0 +1,598 @@ +/* + * libxl_migration.c: methods for handling migration with libxenlight + * + * Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Authors: + * Jim Fehlig jfeh...@suse.com + * Chunyan Liu cy...@suse.com + */ + +#include config.h + +#include internal.h +#include virlog.h +#include virerror.h +#include virconf.h +#include datatypes.h +#include virfile.h +#include viralloc.h +#include viruuid.h +#include vircommand.h +#include virstring.h +#include rpc/virnetsocket.h +#include libxl_domain.h +#include libxl_driver.h +#include libxl_conf.h +#include libxl_migration.h + +#define VIR_FROM_THIS VIR_FROM_LIBXL + +typedef struct _libxlMigrateReceiveArgs { +virConnectPtr conn; +virDomainObjPtr vm; + +/* for freeing listen sockets */ +virNetSocketPtr *socks; +size_t nsocks; +} libxlMigrateReceiveArgs; + +static const char libxlMigrateReceiverReady[] = +libvirt libxl migration receiver ready, send binary domain data; +static const char libxlMigrateReceiverFinish[] = +domain received, ready to unpause; So you're using these to sync source and destination on migration. Kudos for using text protocol not a binary blob. I haven't followed v1
Re: [libvirt] [PATCH v7 0/4] support dumping guest memory in compressed format
Hello, Do you have some comments? On 03/18/2014 03:12 PM, Qiao, Nuohan/乔 诺涵 wrote: dumping guest's memory is introduced without compression supported, but now qemu can dump guest's memory in kdump-compressed format. This patchset is used to add support in libvirt side to let qemu do the dump in compressed format and please refer the following address to see implementation of the qemu side, the lastest version of qemu side is v9. http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg03016.html ChangLog: Changes from v6 to v7: 1. revert changing dumpformat of virDomainCoreDumpWithFormat back to an enum Changes from v5 to v6: 1. add qemuMonitorGetDumpGuestMemoryCapability API to check the available dump format Changes from v4 to v5: 1. modify some restriction check Changes from v3 to v4: 1. dropping patch add dump_memory_format in qemu.conf 2. fix to follow some conventions Changes from v2 to v3: 1. address Jiri Denemark's comment about adding a new public API instead of changing an old one. Changes from v1 to v2: 1. address Daniel P. Berrange's comment about using a new parameter to replace flags like VIR_DUMP_COMPRESS_ZLIB. qiaonuohan (4): add new virDomainCoreDumpWithFormat API qemu: add qemuMonitorGetDumpGuestMemoryCapability qemu: add support for virDomainCoreDumpWithFormat API allow virsh dump --memory-only specify dump format include/libvirt/libvirt.h.in | 31 ++ src/driver.h | 7 src/libvirt.c| 98 src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 70 ++- src/qemu/qemu_monitor.c | 27 ++-- src/qemu/qemu_monitor.h | 6 ++- src/qemu/qemu_monitor_json.c | 69 ++- src/qemu/qemu_monitor_json.h | 6 ++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++- src/remote_protocol-structs | 7 src/test/test_driver.c | 22 -- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 ++-- tools/virsh.pod | 5 +++ 16 files changed, 391 insertions(+), 25 deletions(-) -- Regards Qiao Nuohan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/18] conf: use disk source accessors in bhyve/
Eric Blake wrote: Part of a series of cleanups to use new accessor methods. * src/bhyve/bhyve_command.c (bhyveBuildDiskArgStr) (virBhyveProcessBuildLoadCmd): Use accessors. Signed-off-by: Eric Blake ebl...@redhat.com This patch builds fine for me. I also did a basic testing and didn't notice any regressions. Roman Bogorodskiy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list